git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v5 02/14] trailer: process trailers from file and arguments

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 7, 2014, 00:03 UTC
Message-ID
<xmqqr47fx001.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20140206202004.325.96525.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
Show 90 quoted lines
> This patch implements the logic that process trailers
> from file and arguments.
>
> At the beginning trailers from file are in their own
> infile_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 infile_tok list.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  trailer.c | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 187 insertions(+)
>
> diff --git a/trailer.c b/trailer.c
> index f129b5a..ba0cfe0 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -46,3 +46,190 @@ static inline size_t alnum_len(const char *buf, int len)
>  	while (--len >= 0 && !isalnum(buf[len]));
>  	return (size_t) len + 1;
>  }
> +
> +static void add_arg_to_infile(struct trailer_item *infile_tok,
> +			      struct trailer_item *arg_tok)
> +{
> +	if (arg_tok->conf->where == WHERE_AFTER) {
> +		arg_tok->next = infile_tok->next;
> +		infile_tok->next = arg_tok;
> +		arg_tok->previous = infile_tok;
> +		if (arg_tok->next)
> +			arg_tok->next->previous = arg_tok;
> +	} else {
> +		arg_tok->previous = infile_tok->previous;
> +		infile_tok->previous = arg_tok;
> +		arg_tok->next = infile_tok;
> +		if (arg_tok->previous)
> +			arg_tok->previous->next = arg_tok;
> +	}
> +}
> +
> +static int check_if_different(struct trailer_item *infile_tok,
> +			      struct trailer_item *arg_tok,
> +			      int alnum_len, int check_all)
> +{
> +	enum action_where where = arg_tok->conf->where;
> +	do {
> +		if (!infile_tok)
> +			return 1;
> +		if (same_trailer(infile_tok, arg_tok, alnum_len))
> +			return 0;
> +		/*
> +		 * if we want to add a trailer after another one,
> +		 * we have to check those before this one
> +		 */
> +		infile_tok = (where == WHERE_AFTER) ? infile_tok->previous : infile_tok->next;
> +	} while (check_all);
> +	return 1;
> +}
> +
> +static void apply_arg_if_exist(struct trailer_item *infile_tok,
> +			       struct trailer_item *arg_tok,
> +			       int alnum_len)
> +{
> +	switch (arg_tok->conf->if_exist) {
> +	case EXIST_DO_NOTHING:
> +		free(arg_tok);
> +		break;
> +	case EXIST_OVERWRITE:
> +		free((char *)infile_tok->value);
> +		infile_tok->value = xstrdup(arg_tok->value);
> +		free(arg_tok);
> +		break;
> +	case EXIST_ADD:
> +		add_arg_to_infile(infile_tok, arg_tok);
> +		break;
> +	case EXIST_ADD_IF_DIFFERENT:
> +		if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
> +			add_arg_to_infile(infile_tok, arg_tok);
> +		else
> +			free(arg_tok);
> +		break;
> +	case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
> +		if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
> +			add_arg_to_infile(infile_tok, arg_tok);
> +		else
> +			free(arg_tok);
> +		break;

Makes me wonder if people want a rule to say "if the same key already exists, regardless of the value".

Show 13 quoted lines
> +	}
> +}
> +
> +static void remove_from_list(struct trailer_item *item,
> +			     struct trailer_item **first)
> +{
> +	if (item->next)
> +		item->next->previous = item->previous;
> +	if (item->previous)
> +		item->previous->next = item->next;
> +	else
> +		*first = item->next;
> +}
Will callers free the item that now is not on the list?
Show 53 quoted lines
> +static struct trailer_item *remove_first(struct trailer_item **first)
> +{
> +	struct trailer_item *item = *first;
> +	*first = item->next;
> +	if (item->next) {
> +		item->next->previous = NULL;
> +		item->next = NULL;
> +	}
> +	return item;
> +}
> +
> +static void process_infile_tok(struct trailer_item *infile_tok,
> +			       struct trailer_item **arg_tok_first,
> +			       enum action_where where)
> +{
> +	struct trailer_item *arg_tok;
> +	struct trailer_item *next_arg;
> +
> +	int tok_alnum_len = alnum_len(infile_tok->token, strlen(infile_tok->token));
> +	for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
> +		next_arg = arg_tok->next;
> +		if (same_token(infile_tok, arg_tok, tok_alnum_len) &&
> +		    arg_tok->conf->where == where) {
> +			remove_from_list(arg_tok, arg_tok_first);
> +			apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
> +			/*
> +			 * If arg has been added to infile,
> +			 * then we need to process it too now.
> +			 */
> +			if ((where == WHERE_AFTER ? infile_tok->next : infile_tok->previous) == arg_tok)
> +				infile_tok = arg_tok;
> +		}
> +	}
> +}
> +
> +static void update_last(struct trailer_item **last)
> +{
> +	if (*last)
> +		while((*last)->next != NULL)
> +			*last = (*last)->next;
> +}
> +
> +static void update_first(struct trailer_item **first)
> +{
> +	if (*first)
> +		while((*first)->previous != NULL)
> +			*first = (*first)->previous;
> +}
> +
> +static void apply_arg_if_missing(struct trailer_item **infile_tok_first,
> +				 struct trailer_item **infile_tok_last,
> +				 struct trailer_item *arg_tok)
> +{

Makes me wonder if it would make the code simpler to keep an anchor item "struct trailer_item" that is off heap, and pass that single anchor item around, using its next/prev fields as the first and the last. Wouldn't it let you remove the special cases for the first and last item?

Show 18 quoted lines
> +	struct trailer_item **infile_tok;
> +	enum action_where where;
> +
> +	switch (arg_tok->conf->if_missing) {
> +	case MISSING_DO_NOTHING:
> +		free(arg_tok);
> +		break;
> +	case MISSING_ADD:
> +		where = arg_tok->conf->where;
> +		infile_tok = (where == WHERE_AFTER) ? infile_tok_last : infile_tok_first;
> +		if (*infile_tok) {
> +			add_arg_to_infile(*infile_tok, arg_tok);
> +			*infile_tok = arg_tok;
> +		} else {
> +			*infile_tok_first = arg_tok;
> +			*infile_tok_last = arg_tok;
> +		}
> +		break;

This piece makes me wonder if "after" is a good name. prepend and append, perhaps?

Previous: Christian CouderNext: Christian Couder
Message 9 of 48 in “Add interpret-trailers builtin”
  1. 00/14 Add interpret-trailers builtinChristian Couder, Feb 6, 2014
  2. 01/14 Add data structures and basic functions for commit trailersChristian Couder, Feb 6, 2014
  3. Junio C HamanoFeb 6, 2014
  4. Christian CouderFeb 9, 2014
  5. Eric SunshineFeb 10, 2014
  6. Junio C HamanoFeb 6, 2014
  7. Christian CouderFeb 9, 2014
  8. 02/14 trailer: process trailers from file and argumentsChristian Couder, Feb 6, 2014
  9. Junio C HamanoFeb 7, 2014
  10. Christian CouderFeb 9, 2014
  11. Junio C HamanoFeb 10, 2014
  12. Christian CouderFeb 10, 2014
  13. Junio C HamanoFeb 10, 2014
  14. Christian CouderFeb 11, 2014
  15. Junio C HamanoFeb 11, 2014
  16. Christian CouderFeb 14, 2014
  17. Junio C HamanoFeb 14, 2014
  18. Christian CouderFeb 14, 2014
  19. Junio C HamanoFeb 14, 2014
  20. Junio C HamanoFeb 14, 2014
  21. Junio C HamanoFeb 15, 2014
  22. Junio C HamanoFeb 21, 2014
  23. Christian CouderFeb 23, 2014
  24. 03/14 trailer: read and process config informationChristian Couder, Feb 6, 2014
  25. 04/14 trailer: process command line trailer argumentsChristian Couder, Feb 6, 2014
  26. Junio C HamanoFeb 7, 2014
  27. Christian CouderFeb 9, 2014
  28. 05/14 trailer: parse trailers from input fileChristian Couder, Feb 6, 2014
  29. 06/14 trailer: put all the processing together and printChristian Couder, Feb 6, 2014
  30. 07/14 trailer: add interpret-trailers commandChristian Couder, Feb 6, 2014
  31. Junio C HamanoFeb 7, 2014
  32. Christian CouderFeb 7, 2014
  33. Junio C HamanoFeb 7, 2014
  34. Junio C HamanoFeb 7, 2014
  35. 08/14 trailer: add tests for "git interpret-trailers"Christian Couder, Feb 6, 2014
  36. Junio C HamanoFeb 7, 2014
  37. 09/14 trailer: if no input file is passed, read from stdinChristian Couder, Feb 6, 2014
  38. Junio C HamanoFeb 7, 2014
  39. 10/14 trailer: execute command from 'trailer.<name>.command'Christian Couder, Feb 6, 2014
  40. Junio C HamanoFeb 7, 2014
  41. Junio C HamanoFeb 7, 2014
  42. 11/14 trailer: add tests for trailer commandChristian Couder, Feb 6, 2014
  43. 12/14 trailer: set author and committer env variablesChristian Couder, Feb 6, 2014
  44. Junio C HamanoFeb 7, 2014
  45. Junio C HamanoFeb 7, 2014
  46. 13/14 trailer: add tests for commands using env variablesChristian Couder, Feb 6, 2014
  47. 14/14 Documentation: add documentation for 'git interpret-trailers'Christian Couder, Feb 6, 2014
  48. Eric SunshineFeb 10, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.