Re: [PATCH v4 02/17] trailer: process trailers from file and arguments

2014-01-30 Thread Eric Sunshine
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
chrisc...@tuxfamily.org wrote:
 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 chrisc...@tuxfamily.org
 ---
 diff --git a/trailer.c b/trailer.c
 index aed25e1..e9ccfa5 100644
 --- a/trailer.c
 +++ b/trailer.c
 @@ -46,3 +46,192 @@ static size_t alnum_len(const char *buf, size_t len)
 +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);

This is freeing arg_tok, but isn't it leaking arg_tok-conf, and
conf-name, conf-key, conf-command? Ditto for all the other
free(arg_tok) invocations elsewhere in the file.

  +   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;
 +   }
 +}
 +
 +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 arg_tok from list */
 +   remove_from_list(arg_tok, arg_tok_first);
 +   /* Apply arg */
 +   apply_arg_if_exist(infile_tok, arg_tok, 
 tok_alnum_len);

Redundant comments (saying the same thing as the code) can make the
code slightly more difficult to read.

 +   /*
 +* 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;
 +   }
 +   }
 +}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 02/17] trailer: process trailers from file and arguments

2014-01-29 Thread Christian Couder
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 chrisc...@tuxfamily.org
---
 trailer.c | 189 ++
 1 file changed, 189 insertions(+)

diff --git a/trailer.c b/trailer.c
index aed25e1..e9ccfa5 100644
--- a/trailer.c
+++ b/trailer.c
@@ -46,3 +46,192 @@ static size_t alnum_len(const char *buf, size_t len)
while (--len = 0  !isalnum(buf[len]));
return 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;
+   }
+}
+
+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;
+}
+
+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 arg_tok from list */
+   remove_from_list(arg_tok, arg_tok_first);
+   /* Apply arg */
+   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 ?