Subject: Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments

From: Junio C Hamano <[email-masked]>
Date: Wed, 05 Mar 2014 14:52:47 -0800

Christian Couder <[email-masked]> writes:

> Implement the logic to process trailers from stdin and arguments.
>
> At the beginning trailers from stdin are in their own in_tok
> doubly linked list, and trailers from arguments are in their own
> arg_tok doubly linked list.
>
> The lists are traversed and when an arg_tok should be "applied",
> it is removed from its list and inserted into the in_tok list.
>
> Signed-off-by: Christian Couder <[email-masked]>
> ---

Thanks.

This round is marked as the sixth, but I still see quite a many
style issues, which I expect not to see from long timers without
being told.  Somewhat disappointing...

>  trailer.c | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 197 insertions(+)
>
> diff --git a/trailer.c b/trailer.c
> index db93a63..a0124f2 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -47,3 +47,200 @@ static size_t alnum_len(const char *buf, size_t len)
> ...
> +			if ((where == WHERE_AFTER ? in_tok->next : in_tok->previous) == arg_tok)

Overlong line that does not have to be.

			if ((where == WHERE_AFTER
                             ? in_tok->next : in_tok->previous) == arg_tok)

or something?

> +static void update_last(struct trailer_item **last)
> +{
> +	if (*last)
> +		while((*last)->next != NULL)

Style.  SP between control keyword and the expression.

> +			*last = (*last)->next;
> +}
> +
> +static void update_first(struct trailer_item **first)
> +{
> +	if (*first)
> +		while((*first)->previous != NULL)

Ditto.

> +static void process_trailers_lists(struct trailer_item **in_tok_first,
> ...
> +	/* Process input from end to start */
> +	for (in_tok = *in_tok_last; in_tok; in_tok = in_tok->previous) {
> +		process_input_token(in_tok, arg_tok_first, WHERE_AFTER);
> +	}

Needless brace pair.

> +	/* Process input from start to end */
> +	for (in_tok = *in_tok_first; in_tok; in_tok = in_tok->next) {
> +		process_input_token(in_tok, arg_tok_first, WHERE_BEFORE);
> +	}

Ditto.