Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments
On Fri, Mar 07, 2014 at 07:19:15AM +0100, Christian Couder wrote: On Wed, Mar 5, 2014 at 11:52 PM, Junio C Hamano gits...@pobox.com wrote: 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... Yeah, I don't know why, but these days I find it very hard to review style issues in my own code without being distracted. And by the way is there a good script to check them? Many of these would have been caught with kernel.org's checkpatch.pl script. http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl regards, dan carpenter -- 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
Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments
On Fri, Mar 7, 2014 at 9:08 AM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Mar 07, 2014 at 07:19:15AM +0100, Christian Couder wrote: Yeah, I don't know why, but these days I find it very hard to review style issues in my own code without being distracted. And by the way is there a good script to check them? Many of these would have been caught with kernel.org's checkpatch.pl script. http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl Yeah, I already thought about using it in the past, but I was not sure about its status regarding the git source code because it's use is not suggested in CodingGuidelines, and the discussions about adding it there went nowhere: http://thread.gmane.org/gmane.comp.version-control.git/223698/focus=224653 Thanks, Christian. -- 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
Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments
On Wed, Mar 5, 2014 at 11:52 PM, Junio C Hamano gits...@pobox.com wrote: 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... Yeah, I don't know why, but these days I find it very hard to review style issues in my own code without being distracted. And by the way is there a good script to check them? Thanks, Christian. -- 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
Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments
Christian Couder chrisc...@tuxfamily.org 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 chrisc...@tuxfamily.org
---
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.
--
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 v6 02/11] trailer: process trailers from stdin and arguments
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 chrisc...@tuxfamily.org
---
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)
len--;
return len;
}
+
+static void free_trailer_item(struct trailer_item *item)
+{
+ free(item-conf.name);
+ free(item-conf.key);
+ free(item-conf.command);
+ free((char *)item-token);
+ free((char *)item-value);
+ free(item);
+}
+
+static void add_arg_to_input_list(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf.where == WHERE_AFTER) {
+ arg_tok-next = in_tok-next;
+ in_tok-next = arg_tok;
+ arg_tok-previous = in_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = in_tok-previous;
+ in_tok-previous = arg_tok;
+ arg_tok-next = in_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf.where;
+ do {
+ if (!in_tok)
+ return 1;
+ if (same_trailer(in_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
+*/
+ in_tok = (where == WHERE_AFTER) ? in_tok-previous :
in_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exists(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len)
+{
+ switch (arg_tok-conf.if_exists) {
+ case EXISTS_DO_NOTHING:
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_OVERWRITE:
+ free((char *)in_tok-value);
+ in_tok-value = xstrdup(arg_tok-value);
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD:
+ add_arg_to_input_list(in_tok, arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 1))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 0))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(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_input_token(struct trailer_item *in_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(in_tok-token, strlen(in_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(in_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf.where == where) {
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exists(in_tok, arg_tok, tok_alnum_len);
+ /*
+* If arg has been added to input,
+* then we need to process it too
