{"thread":{"id":"28384","subject":"[PATCH 0/7] create argv_array API","startedAt":"2011-09-13T21:50:26Z","lastAt":"2011-09-14T23:18:04Z","messageCount":18,"participants":["Jeff King","Christian Couder","Andreas Ericsson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"175421","messageId":"20110913215026.GA26743@sigill.intra.peff.net","threadId":"28384","inReplyTo":null,"subject":"[PATCH 0/7] create argv_array API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:50:26Z","receivedAt":"2011-09-13T21:50:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 13, 2011 at 10:34:58AM -0700, Junio C Hamano wrote:\n\n> > Will do. Junio, do you want me to re-roll the quadratic fix, or just\n> > build the refactoring on top?\n> \n> The latter would be fine and it probably is not even urgent.\n\nI went ahead and did it now, while it is still in my head. The first two\npatches are unrelated fixups I noticed while working on it. The third is\nthe refactoring, and then the rest use it in various places. They're\ncertainly not urgent, and the final one borders on code churn, so they\nmay not all be worth applying.  But I don't think they conflict with\nanything in 'next', so now might be a good time.\n\nThese are on top of what you have in jk/maint-fetch-submodule-check-fix.\n\n  [1/7]: add sha1_array API docs\n  [2/7]: quote.h: fix bogus comment\n  [3/7]: refactor argv_array into generic code\n  [4/7]: quote: provide sq_dequote_to_argv_array\n  [5/7]: bisect: use argv_array API\n  [6/7]: checkout: use argv_array API\n  [7/7]: run_hook: use argv_array API\n\n-Peff\n"},{"id":"175422","messageId":"20110913215733.GA24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 1/7] add sha1_array API docs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:57:34Z","receivedAt":"2011-09-13T21:57:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This API was introduced in 902bb36, but never documented.\nLet's be nice to future users of the code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/technical/api-sha1-array.txt |   79 ++++++++++++++++++++++++++++\n 1 files changed, 79 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/technical/api-sha1-array.txt\n\ndiff --git a/Documentation/technical/api-sha1-array.txt b/Documentation/technical/api-sha1-array.txt\nnew file mode 100644\nindex 0000000..4a4bae8\n--- /dev/null\n+++ b/Documentation/technical/api-sha1-array.txt\n@@ -0,0 +1,79 @@\n+sha1-array API\n+==============\n+\n+The sha1-array API provides storage and manipulation of sets of SHA1\n+identifiers. The emphasis is on storage and processing efficiency,\n+making them suitable for large lists. Note that the ordering of items is\n+not preserved over some operations.\n+\n+Data Structures\n+---------------\n+\n+`struct sha1_array`::\n+\n+\tA single array of SHA1 hashes. This should be initialized by\n+\tassignment from `SHA1_ARRAY_INIT`.  The `sha1` member contains\n+\tthe actual data. The `nr` member contains the number of items in\n+\tthe set.  The `alloc` and `sorted` members are used internally,\n+\tand should not be needed by API callers.\n+\n+Functions\n+---------\n+\n+`sha1_array_append`::\n+\tAdd an item to the set. The sha1 will be placed at the end of\n+\tthe array (but note that some operations below may lose this\n+\tordering).\n+\n+`sha1_array_sort`::\n+\tSort the elements in the array.\n+\n+`sha1_array_lookup`::\n+\tPerform a binary search of the array for a specific sha1.\n+\tIf found, returns the offset (in number of elements) of the\n+\tsha1. If not found, returns a negative integer. If the array is\n+\tnot sorted, this function has the side effect of sorting it.\n+\n+`sha1_array_clear`::\n+\tFree all memory associated with the array and return it to the\n+\tinitial, empty state.\n+\n+`sha1_array_for_each_unique`::\n+\tEfficiently iterate over each unique element of the list,\n+\texecuting the callback function for each one. If the array is\n+\tnot sorted, this function has the side effect of sorting it.\n+\n+Examples\n+--------\n+\n+-----------------------------------------\n+void print_callback(const unsigned char sha1[20],\n+\t\t    void *data)\n+{\n+\tprintf(\"%s\\n\", sha1_to_hex(sha1));\n+}\n+\n+void some_func(void)\n+{\n+\tstruct sha1_array hashes = SHA1_ARRAY_INIT;\n+\tunsigned char sha1[20];\n+\n+\t/* Read objects into our set */\n+\twhile (read_object_from_stdin(sha1))\n+\t\tsha1_array_append(&hashes, sha1);\n+\n+\t/* Check if some objects are in our set */\n+\twhile (read_object_from_stdin(sha1)) {\n+\t\tif (sha1_array_lookup(&hashes, sha1) >= 0)\n+\t\t\tprintf(\"it's in there!\\n\");\n+\n+\t/*\n+\t * Print the unique set of objects. We could also have\n+\t * avoided adding duplicate objects in the first place,\n+\t * but we would end up re-sorting the array repeatedly.\n+\t * Instead, this will sort once and then skip duplicates\n+\t * in linear time.\n+\t */\n+\tsha1_array_for_each_unique(&hashes, print_callback, NULL);\n+}\n+-----------------------------------------\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175423","messageId":"20110913215747.GB24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 2/7] quote.h: fix bogus comment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:57:47Z","receivedAt":"2011-09-13T21:57:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 758e915 made sq_quote_next static, removing it from\nquote.h. However, it forgot to update the related comment,\nmaking it appear as a confusing description of sq_quote_to_argv.\n\nLet's remove the crufty bits, and elaborate more on sq_quote_to_argv.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n quote.h |    5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/quote.h b/quote.h\nindex 024e21d..252b0df 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -40,9 +40,8 @@\n \n /*\n  * Same as the above, but can be used to unwrap many arguments in the\n- * same string separated by space. \"next\" is changed to point to the\n- * next argument that should be passed as first parameter. When there\n- * is no more argument to be dequoted, \"next\" is updated to point to NULL.\n+ * same string separated by space. Like sq_quote, it works in place,\n+ * modifying arg and appending pointers into it to argv.\n  */\n extern int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);\n \n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175424","messageId":"20110913215757.GC24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 3/7] refactor argv_array into generic code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:57:57Z","receivedAt":"2011-09-13T21:57:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The submodule code recently grew generic code to build a\ndynamic argv array. Many other parts of the code can reuse\nthis, too, so let's make it generically available.\n\nThere are two enhancements not found in the original code:\n\n  1. We now handle the NULL-termination invariant properly,\n     even when no strings have been pushed (before, you\n     could have an empty, NULL argv). This was not a problem\n     for the submodule code, which always pushed at least\n     one argument, but was not sufficiently safe for\n     generic code.\n\n  2. There is a formatted variant of the \"push\" function.\n     This is a convenience function which was not needed by\n     the submodule code, but will make it easier to port\n     other users to the new code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/technical/api-argv-array.txt |   46 ++++++++++++++++++++++++\n Makefile                                   |    2 +\n argv-array.c                               |   52 ++++++++++++++++++++++++++++\n argv-array.h                               |   20 +++++++++++\n submodule.c                                |   41 +++-------------------\n 5 files changed, 126 insertions(+), 35 deletions(-)\n create mode 100644 Documentation/technical/api-argv-array.txt\n create mode 100644 argv-array.c\n create mode 100644 argv-array.h\n\ndiff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt\nnew file mode 100644\nindex 0000000..49b3d52\n--- /dev/null\n+++ b/Documentation/technical/api-argv-array.txt\n@@ -0,0 +1,46 @@\n+argv-array API\n+==============\n+\n+The argv-array API allows one to dynamically build and store\n+NULL-terminated lists.  An argv-array maintains the invariant that the\n+`argv` member always points to a non-NULL array, and that the array is\n+always NULL-terminated at the element pointed to by `argv[argc]`. This\n+makes the result suitable for passing to functions expecting to receive\n+argv from main(), or the link:api-run-command.html[run-command API].\n+\n+The link:api-string-list.html[string-list API] is similar, but cannot be\n+used for these purposes; instead of storing a straight string pointer,\n+it contains an item structure with a `util` field that is not compatible\n+with the traditional argv interface.\n+\n+Each `argv_array` manages its own memory. Any strings pushed into the\n+array are duplicated, and all memory is freed by argv_array_clear().\n+\n+Data Structures\n+---------------\n+\n+`struct argv_array`::\n+\n+\tA single array. This should be initialized by assignment from\n+\t`ARGV_ARRAY_INIT`, or by calling `argv_array_init`. The `argv`\n+\tmember contains the actual array; the `argc` member contains the\n+\tnumber of elements in the array, not including the terminating\n+\tNULL.\n+\n+Functions\n+---------\n+\n+`argv_array_init`::\n+\tInitialize an array. This is no different than assigning from\n+\t`ARGV_ARRAY_INIT`.\n+\n+`argv_array_push`::\n+\tPush a copy of a string onto the end of the array.\n+\n+`argv_array_pushf`::\n+\tFormat a string and push it onto the end of the array. This is a\n+\tconvenience wrapper combining `strbuf_addf` and `argv_array_push`.\n+\n+`argv_array_clear`::\n+\tFree all memory associated with the array and return it to the\n+\tinitial, empty state.\ndiff --git a/Makefile b/Makefile\nindex e40ac0c..ec7d06c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -497,6 +497,7 @@ VCSSVN_LIB=vcs-svn/lib.a\n \n LIB_H += advice.h\n LIB_H += archive.h\n+LIB_H += argv-array.h\n LIB_H += attr.h\n LIB_H += blob.h\n LIB_H += builtin.h\n@@ -575,6 +576,7 @@ LIB_OBJS += alloc.o\n LIB_OBJS += archive.o\n LIB_OBJS += archive-tar.o\n LIB_OBJS += archive-zip.o\n+LIB_OBJS += argv-array.o\n LIB_OBJS += attr.o\n LIB_OBJS += base85.o\n LIB_OBJS += bisect.o\ndiff --git a/argv-array.c b/argv-array.c\nnew file mode 100644\nindex 0000000..a50507a\n--- /dev/null\n+++ b/argv-array.c\n@@ -0,0 +1,52 @@\n+#include \"cache.h\"\n+#include \"argv-array.h\"\n+#include \"strbuf.h\"\n+\n+static const char *empty_argv_storage = NULL;\n+const char **empty_argv = &empty_argv_storage;\n+\n+void argv_array_init(struct argv_array *array)\n+{\n+\tarray->argv = empty_argv;\n+\tarray->argc = 0;\n+\tarray->alloc = 0;\n+}\n+\n+static void argv_array_push_nodup(struct argv_array *array, const char *value)\n+{\n+\tif (array->argv == empty_argv)\n+\t\tarray->argv = NULL;\n+\n+\tALLOC_GROW(array->argv, array->argc + 2, array->alloc);\n+\tarray->argv[array->argc++] = value;\n+\tarray->argv[array->argc] = NULL;\n+}\n+\n+void argv_array_push(struct argv_array *array, const char *value)\n+{\n+\targv_array_push_nodup(array, xstrdup(value));\n+}\n+\n+void argv_array_pushf(struct argv_array *array, const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tstruct strbuf v = STRBUF_INIT;\n+\n+\tva_start(ap, fmt);\n+\tstrbuf_vaddf(&v, fmt, ap);\n+\tva_end(ap);\n+\n+\targv_array_push_nodup(array, strbuf_detach(&v, NULL));\n+}\n+\n+void argv_array_clear(struct argv_array *array)\n+{\n+\tif (array->argv != empty_argv) {\n+\t\tint i;\n+\t\tfor (i = 0; i < array->argc; i++)\n+\t\t\tfree((char **)array->argv[i]);\n+\t\tfree(array->argv);\n+\t}\n+\targv_array_init(array);\n+}\n+\ndiff --git a/argv-array.h b/argv-array.h\nnew file mode 100644\nindex 0000000..2b6273b\n--- /dev/null\n+++ b/argv-array.h\n@@ -0,0 +1,20 @@\n+#ifndef ARGV_ARRAY_H\n+#define ARGV_ARRAY_H\n+\n+extern const char **empty_argv;\n+\n+struct argv_array {\n+\tconst char **argv;\n+\tint argc;\n+\tint alloc;\n+};\n+\n+#define ARGV_ARRAY_INIT { empty_argv, 0, 0 };\n+\n+void argv_array_init(struct argv_array *);\n+void argv_array_push(struct argv_array *, const char *);\n+__attribute__((format (printf,2,3)))\n+void argv_array_pushf(struct argv_array *, const char *fmt, ...);\n+void argv_array_clear(struct argv_array *);\n+\n+#endif /* ARGV_ARRAY_H */\ndiff --git a/submodule.c b/submodule.c\nindex 9431c42..6306737 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -9,6 +9,7 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n+#include \"argv-array.h\"\n \n static struct string_list config_name_for_path;\n static struct string_list config_fetch_recurse_submodules_for_name;\n@@ -388,52 +389,22 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])\n \tsha1_array_append(&ref_tips_after_fetch, new_sha1);\n }\n \n-struct argv_array {\n-\tconst char **argv;\n-\tunsigned int argc;\n-\tunsigned int alloc;\n-};\n-\n-static void init_argv(struct argv_array *array)\n-{\n-\tarray->argv = NULL;\n-\tarray->argc = 0;\n-\tarray->alloc = 0;\n-}\n-\n-static void push_argv(struct argv_array *array, const char *value)\n-{\n-\tALLOC_GROW(array->argv, array->argc + 2, array->alloc);\n-\tarray->argv[array->argc++] = xstrdup(value);\n-\tarray->argv[array->argc] = NULL;\n-}\n-\n-static void clear_argv(struct argv_array *array)\n-{\n-\tint i;\n-\tfor (i = 0; i < array->argc; i++)\n-\t\tfree((char **)array->argv[i]);\n-\tfree(array->argv);\n-\tinit_argv(array);\n-}\n-\n static void add_sha1_to_argv(const unsigned char sha1[20], void *data)\n {\n-\tpush_argv(data, sha1_to_hex(sha1));\n+\targv_array_push(data, sha1_to_hex(sha1));\n }\n \n static void calculate_changed_submodule_paths(void)\n {\n \tstruct rev_info rev;\n \tstruct commit *commit;\n-\tstruct argv_array argv;\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n \tinit_revisions(&rev, NULL);\n-\tinit_argv(&argv);\n-\tpush_argv(&argv, \"--\"); /* argv[0] program name */\n+\targv_array_push(&argv, \"--\"); /* argv[0] program name */\n \tsha1_array_for_each_unique(&ref_tips_after_fetch,\n \t\t\t\t   add_sha1_to_argv, &argv);\n-\tpush_argv(&argv, \"--not\");\n+\targv_array_push(&argv, \"--not\");\n \tsha1_array_for_each_unique(&ref_tips_before_fetch,\n \t\t\t\t   add_sha1_to_argv, &argv);\n \tsetup_revisions(argv.argc, argv.argv, &rev, NULL);\n@@ -460,7 +431,7 @@ static void calculate_changed_submodule_paths(void)\n \t\t}\n \t}\n \n-\tclear_argv(&argv);\n+\targv_array_clear(&argv);\n \tsha1_array_clear(&ref_tips_before_fetch);\n \tsha1_array_clear(&ref_tips_after_fetch);\n \tinitialized_fetch_ref_tips = 0;\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175425","messageId":"20110913215808.GD24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 4/7] quote: provide sq_dequote_to_argv_array","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:58:08Z","receivedAt":"2011-09-13T21:58:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is similar to sq_dequote_to_argv, but more convenient\nif you have an argv_array. It's tempting to just feed the\ncomponents of the argv_array to sq_dequote_to_argv instead,\nbut:\n\n  1. It wouldn't maintain the NULL-termination invariant\n     of argv_array.\n\n  2. It doesn't match the memory ownership policy of\n     argv_array (in which each component is free-able, not a\n     pointer into a separate buffer).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n quote.c |   23 ++++++++++++++++++++---\n quote.h |    8 ++++++++\n 2 files changed, 28 insertions(+), 3 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex 63d3b01..87bc65e 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"quote.h\"\n+#include \"argv-array.h\"\n \n int quote_path_fully = 1;\n \n@@ -120,7 +121,9 @@ void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)\n \treturn sq_dequote_step(arg, NULL);\n }\n \n-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)\n+static int sq_dequote_to_argv_internal(char *arg,\n+\t\t\t\t       const char ***argv, int *nr, int *alloc,\n+\t\t\t\t       struct argv_array *array)\n {\n \tchar *next = arg;\n \n@@ -130,13 +133,27 @@ int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)\n \t\tchar *dequoted = sq_dequote_step(next, &next);\n \t\tif (!dequoted)\n \t\t\treturn -1;\n-\t\tALLOC_GROW(*argv, *nr + 1, *alloc);\n-\t\t(*argv)[(*nr)++] = dequoted;\n+\t\tif (argv) {\n+\t\t\tALLOC_GROW(*argv, *nr + 1, *alloc);\n+\t\t\t(*argv)[(*nr)++] = dequoted;\n+\t\t}\n+\t\tif (array)\n+\t\t\targv_array_push(array, dequoted);\n \t} while (next);\n \n \treturn 0;\n }\n \n+int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)\n+{\n+\treturn sq_dequote_to_argv_internal(arg, argv, nr, alloc, NULL);\n+}\n+\n+int sq_dequote_to_argv_array(char *arg, struct argv_array *array)\n+{\n+\treturn sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);\n+}\n+\n /* 1 means: quote as octal\n  * 0 means: quote as octal if (quote_path_fully)\n  * -1 means: never quote\ndiff --git a/quote.h b/quote.h\nindex 252b0df..133155a 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -45,6 +45,14 @@\n  */\n extern int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);\n \n+/*\n+ * Same as above, but store the unquoted strings in an argv_array. We will\n+ * still modify arg in place, but unlike sq_dequote_to_argv, the argv_array\n+ * will duplicate and take ownership of the strings.\n+ */\n+struct argv_array;\n+extern int sq_dequote_to_argv_array(char *arg, struct argv_array *);\n+\n extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);\n extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175426","messageId":"20110913215814.GE24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 5/7] bisect: use argv_array API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:58:14Z","receivedAt":"2011-09-13T21:58:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that the argv_array API exists, we can save some lines\nof code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n bisect.c |   48 +++++++++++-------------------------------------\n 1 files changed, 11 insertions(+), 37 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex dd7e8ed..ef92871 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -10,18 +10,13 @@\n #include \"log-tree.h\"\n #include \"bisect.h\"\n #include \"sha1-array.h\"\n+#include \"argv-array.h\"\n \n static struct sha1_array good_revs;\n static struct sha1_array skipped_revs;\n \n static const unsigned char *current_bad_sha1;\n \n-struct argv_array {\n-\tconst char **argv;\n-\tint argv_nr;\n-\tint argv_alloc;\n-};\n-\n static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n \n@@ -404,21 +399,6 @@ struct commit_list *find_bisection(struct commit_list *list,\n \treturn best;\n }\n \n-static void argv_array_push(struct argv_array *array, const char *string)\n-{\n-\tALLOC_GROW(array->argv, array->argv_nr + 1, array->argv_alloc);\n-\tarray->argv[array->argv_nr++] = string;\n-}\n-\n-static void argv_array_push_sha1(struct argv_array *array,\n-\t\t\t\t const unsigned char *sha1,\n-\t\t\t\t const char *format)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tstrbuf_addf(&buf, format, sha1_to_hex(sha1));\n-\targv_array_push(array, strbuf_detach(&buf, NULL));\n-}\n-\n static int register_ref(const char *refname, const unsigned char *sha1,\n \t\t\tint flags, void *cb_data)\n {\n@@ -448,16 +428,10 @@ static void read_bisect_paths(struct argv_array *array)\n \t\tdie_errno(\"Could not open file '%s'\", filename);\n \n \twhile (strbuf_getline(&str, fp, '\\n') != EOF) {\n-\t\tchar *quoted;\n-\t\tint res;\n-\n \t\tstrbuf_trim(&str);\n-\t\tquoted = strbuf_detach(&str, NULL);\n-\t\tres = sq_dequote_to_argv(quoted, &array->argv,\n-\t\t\t\t\t &array->argv_nr, &array->argv_alloc);\n-\t\tif (res)\n+\t\tif (sq_dequote_to_argv_array(str.buf, array))\n \t\t\tdie(\"Badly quoted content in file '%s': %s\",\n-\t\t\t    filename, quoted);\n+\t\t\t    filename, str.buf);\n \t}\n \n \tstrbuf_release(&str);\n@@ -622,7 +596,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,\n \t\t\t     const char *bad_format, const char *good_format,\n \t\t\t     int read_paths)\n {\n-\tstruct argv_array rev_argv = { NULL, 0, 0 };\n+\tstruct argv_array rev_argv = ARGV_ARRAY_INIT;\n \tint i;\n \n \tinit_revisions(revs, prefix);\n@@ -630,17 +604,17 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,\n \trevs->commit_format = CMIT_FMT_UNSPECIFIED;\n \n \t/* rev_argv.argv[0] will be ignored by setup_revisions */\n-\targv_array_push(&rev_argv, xstrdup(\"bisect_rev_setup\"));\n-\targv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format);\n+\targv_array_push(&rev_argv, \"bisect_rev_setup\");\n+\targv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1));\n \tfor (i = 0; i < good_revs.nr; i++)\n-\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i],\n-\t\t\t\t     good_format);\n-\targv_array_push(&rev_argv, xstrdup(\"--\"));\n+\t\targv_array_pushf(&rev_argv, good_format,\n+\t\t\t\t sha1_to_hex(good_revs.sha1[i]));\n+\targv_array_push(&rev_argv, \"--\");\n \tif (read_paths)\n \t\tread_bisect_paths(&rev_argv);\n-\targv_array_push(&rev_argv, NULL);\n \n-\tsetup_revisions(rev_argv.argv_nr, rev_argv.argv, revs, NULL);\n+\tsetup_revisions(rev_argv.argc, rev_argv.argv, revs, NULL);\n+\t/* XXX leak rev_argv, as \"revs\" may still be pointing to it */\n }\n \n static void bisect_common(struct rev_info *revs)\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175428","messageId":"20110913215819.GF24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 6/7] checkout: use argv_array API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:58:19Z","receivedAt":"2011-09-13T21:58:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We were using a similar ad-hoc rev_list_args structure, but\nthis saves some code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/checkout.c |   27 ++++++++-------------------\n 1 files changed, 8 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 28cdc51..bb056e4 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -19,6 +19,7 @@\n #include \"ll-merge.h\"\n #include \"resolve-undo.h\"\n #include \"submodule.h\"\n+#include \"argv-array.h\"\n \n static const char * const checkout_usage[] = {\n \t\"git checkout [options] <branch>\",\n@@ -588,24 +589,12 @@ static void update_refs_for_switch(struct checkout_opts *opts,\n \t\treport_tracking(new);\n }\n \n-struct rev_list_args {\n-\tint argc;\n-\tint alloc;\n-\tconst char **argv;\n-};\n-\n-static void add_one_rev_list_arg(struct rev_list_args *args, const char *s)\n-{\n-\tALLOC_GROW(args->argv, args->argc + 1, args->alloc);\n-\targs->argv[args->argc++] = s;\n-}\n-\n static int add_one_ref_to_rev_list_arg(const char *refname,\n \t\t\t\t       const unsigned char *sha1,\n \t\t\t\t       int flags,\n \t\t\t\t       void *cb_data)\n {\n-\tadd_one_rev_list_arg(cb_data, refname);\n+\targv_array_push(cb_data, refname);\n \treturn 0;\n }\n \n@@ -684,15 +673,14 @@ static void suggest_reattach(struct commit *commit, struct rev_info *revs)\n  */\n static void orphaned_commit_warning(struct commit *commit)\n {\n-\tstruct rev_list_args args = { 0, 0, NULL };\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n \tstruct rev_info revs;\n \n-\tadd_one_rev_list_arg(&args, \"(internal)\");\n-\tadd_one_rev_list_arg(&args, sha1_to_hex(commit->object.sha1));\n-\tadd_one_rev_list_arg(&args, \"--not\");\n+\targv_array_push(&args, \"(internal)\");\n+\targv_array_push(&args, sha1_to_hex(commit->object.sha1));\n+\targv_array_push(&args, \"--not\");\n \tfor_each_ref(add_one_ref_to_rev_list_arg, &args);\n-\tadd_one_rev_list_arg(&args, \"--\");\n-\tadd_one_rev_list_arg(&args, NULL);\n+\targv_array_push(&args, \"--\");\n \n \tinit_revisions(&revs, NULL);\n \tif (setup_revisions(args.argc - 1, args.argv, &revs, NULL) != 1)\n@@ -704,6 +692,7 @@ static void orphaned_commit_warning(struct commit *commit)\n \telse\n \t\tdescribe_detached_head(_(\"Previous HEAD position was\"), commit);\n \n+\targv_array_clear(&args);\n \tclear_commit_marks(commit, -1);\n \tfor_each_ref(clear_commit_marks_from_one_ref, NULL);\n }\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175427","messageId":"20110913215824.GG24490@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215026.GA26743@sigill.intra.peff.net","subject":"[PATCH 7/7] run_hook: use argv_array API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T21:58:25Z","receivedAt":"2011-09-13T21:58:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was a pretty straightforward use, so it really doesn't\nsave that many lines. Still, perhaps it's a little bit more\nreadable.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n run-command.c |   18 ++++++++----------\n 1 files changed, 8 insertions(+), 10 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 70e8a24..73e013e 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n #include \"exec_cmd.h\"\n+#include \"argv-array.h\"\n \n static inline void close_pair(int fd[2])\n {\n@@ -609,26 +610,23 @@ int finish_async(struct async *async)\n int run_hook(const char *index_file, const char *name, ...)\n {\n \tstruct child_process hook;\n-\tconst char **argv = NULL, *env[2];\n+\tstruct argv_array argv = ARGV_ARRAY_INIT;\n+\tconst char *p, *env[2];\n \tchar index[PATH_MAX];\n \tva_list args;\n \tint ret;\n-\tsize_t i = 0, alloc = 0;\n \n \tif (access(git_path(\"hooks/%s\", name), X_OK) < 0)\n \t\treturn 0;\n \n \tva_start(args, name);\n-\tALLOC_GROW(argv, i + 1, alloc);\n-\targv[i++] = git_path(\"hooks/%s\", name);\n-\twhile (argv[i-1]) {\n-\t\tALLOC_GROW(argv, i + 1, alloc);\n-\t\targv[i++] = va_arg(args, const char *);\n-\t}\n+\targv_array_push(&argv, git_path(\"hooks/%s\", name));\n+\twhile ((p = va_arg(args, const char *)))\n+\t\targv_array_push(&argv, p);\n \tva_end(args);\n \n \tmemset(&hook, 0, sizeof(hook));\n-\thook.argv = argv;\n+\thook.argv = argv.argv;\n \thook.no_stdin = 1;\n \thook.stdout_to_stderr = 1;\n \tif (index_file) {\n@@ -639,6 +637,6 @@ int run_hook(const char *index_file, const char *name, ...)\n \t}\n \n \tret = run_command(&hook);\n-\tfree(argv);\n+\targv_array_clear(&argv);\n \treturn ret;\n }\n-- \n1.7.7.rc1.2.gb2409\n"},{"id":"175429","messageId":"20110913220421.GA24549@sigill.intra.peff.net","threadId":"28384","inReplyTo":"20110913215824.GG24490@sigill.intra.peff.net","subject":"[nit] diff func headers ignore context","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-13T22:04:21Z","receivedAt":"2011-09-13T22:04:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:\n\n> @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n>  int run_hook(const char *index_file, const char *name, ...)\n>  {\n>  \tstruct child_process hook;\n> -\tconst char **argv = NULL, *env[2];\n> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n\nI find this diff function header pretty confusing. Of course we're not\nin finish_async, as you can see by the fact that the context contains\nthe start of run_hook.\n\nI don't think this is something that can be solved with xfuncname\nconfig; we would have to teach xdiff to look at context lines when\npicking a header line.\n\nAm I the only one who finds this confusing? Can anyone think of a reason\nto keep showing finish_async in this example?\n\n-Peff\n"},{"id":"175452","messageId":"CAP8UFD1vxP9ABgJpM99hxDWWLeGO_QW7QLVFq1f-teu1fiCftA@mail.gmail.com","threadId":"28384","inReplyTo":"20110913215757.GC24490@sigill.intra.peff.net","subject":"Re: [PATCH 3/7] refactor argv_array into generic code","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2011-09-14T05:54:48Z","receivedAt":"2011-09-14T05:54:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Peff,\n\nOn Tue, Sep 13, 2011 at 11:57 PM, Jeff King <peff@peff.net> wrote:\n> diff --git a/argv-array.c b/argv-array.c\n> new file mode 100644\n> index 0000000..a50507a\n> --- /dev/null\n> +++ b/argv-array.c\n> @@ -0,0 +1,52 @@\n> +#include \"cache.h\"\n> +#include \"argv-array.h\"\n> +#include \"strbuf.h\"\n> +\n> +static const char *empty_argv_storage = NULL;\n> +const char **empty_argv = &empty_argv_storage;\n> +\n> +void argv_array_init(struct argv_array *array)\n> +{\n> +       array->argv = empty_argv;\n> +       array->argc = 0;\n> +       array->alloc = 0;\n> +}\n> +\n> +static void argv_array_push_nodup(struct argv_array *array, const char *value)\n> +{\n> +       if (array->argv == empty_argv)\n> +               array->argv = NULL;\n> +\n> +       ALLOC_GROW(array->argv, array->argc + 2, array->alloc);\n> +       array->argv[array->argc++] = value;\n> +       array->argv[array->argc] = NULL;\n> +}\n> +\n> +void argv_array_push(struct argv_array *array, const char *value)\n> +{\n> +       argv_array_push_nodup(array, xstrdup(value));\n> +}\n> +\n> +void argv_array_pushf(struct argv_array *array, const char *fmt, ...)\n> +{\n> +       va_list ap;\n> +       struct strbuf v = STRBUF_INIT;\n> +\n> +       va_start(ap, fmt);\n> +       strbuf_vaddf(&v, fmt, ap);\n> +       va_end(ap);\n> +\n> +       argv_array_push_nodup(array, strbuf_detach(&v, NULL));\n> +}\n\nIn sha1-array you called the \"push\" function \"sha1_array_append\"\ninstead of \"sha1_array_push\", so I wonder why here you call them\n\"*_push*\" instead of \"*_append*\"?\n\nThanks for doing this anyway,\nChristian.\n"},{"id":"175462","messageId":"4E707E45.5070102@op5.se","threadId":"28384","inReplyTo":"20110913220421.GA24549@sigill.intra.peff.net","subject":"Re: [nit] diff func headers ignore context","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2011-09-14T10:13:25Z","receivedAt":"2011-09-14T10:13:25Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 09/14/2011 12:04 AM, Jeff King wrote:\n> On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:\n> \n>> @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n>>   int run_hook(const char *index_file, const char *name, ...)\n>>   {\n>>   \tstruct child_process hook;\n>> -\tconst char **argv = NULL, *env[2];\n>> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> \n> I find this diff function header pretty confusing. Of course we're not\n> in finish_async, as you can see by the fact that the context contains\n> the start of run_hook.\n> \n> I don't think this is something that can be solved with xfuncname\n> config; we would have to teach xdiff to look at context lines when\n> picking a header line.\n> \n> Am I the only one who finds this confusing? Can anyone think of a reason\n> to keep showing finish_async in this example?\n> \n\nI've never thought about the problem as I tend to read changes and\ncontext before header.\n\nCurrent behaviour makes sense if the closing brace of the previous\nfunction is inside the context, so if we're going to change this,\nwe'd either have to teach xfuncname() (or whatever we'll be using)\nabout indentation to make it handle bannerstyle[1] indentation, or\ntake any closing brace before a new first-level indentation as a\nsign to close the previous function. OTOH we could just ignore\nbannerstyle indent, as it's sort of bizarre and not very widely\nused, but we'd still have to handle the case of the previous\nfunction being closed inside the context.\n\nPersonally, I don't think it's worth it, but I certainly see your\npoint.\n\n\n*1. Bannerstyle indent looks like this, for those who aren't\n    familiar with this very confusing style.\nint prefixcmp(char *haystack, char *needle) {\n\treturn strncmp(haystack, needle, strlen(needle));\n\t}\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"175491","messageId":"7v4o0fufsn.fsf@alter.siamese.dyndns.org","threadId":"28384","inReplyTo":"20110913215757.GC24490@sigill.intra.peff.net","subject":"Re: [PATCH 3/7] refactor argv_array into generic code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T18:42:16Z","receivedAt":"2011-09-14T18:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The submodule code recently grew generic code to build a\n> dynamic argv array.\n\nThat has not yet happened as far as I am concerned X-<.\n\nI do not want the clean-up to depend on the uncooked submodule code and\nthat was why I said this is not urgent.\n\nWill think about what to do next. Eh, rather, will backburner thinking\nabout it for now ;-).\n"},{"id":"175494","messageId":"7vy5xrt0ti.fsf@alter.siamese.dyndns.org","threadId":"28384","inReplyTo":"7v4o0fufsn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/7] refactor argv_array into generic code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T18:51:05Z","receivedAt":"2011-09-14T18:51:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> The submodule code recently grew generic code to build a\n>> dynamic argv array.\n>\n> That has not yet happened as far as I am concerned X-<.\n>\n> I do not want the clean-up to depend on the uncooked submodule code and\n> that was why I said this is not urgent.\n>\n> Will think about what to do next. Eh, rather, will backburner thinking\n> about it for now ;-).\n\nAhh, I thought this \"submodule\" was about enhancing the on-demand stuff,\nbut you meant your fix to the \"other half\". I should have looked before\nresponding; sorry.\n\nWill queue directly on top of that fix and all will be well.\n\nThanks.\n"},{"id":"175495","messageId":"7vty8ft0oc.fsf@alter.siamese.dyndns.org","threadId":"28384","inReplyTo":"20110913215824.GG24490@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] run_hook: use argv_array API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T18:54:11Z","receivedAt":"2011-09-14T18:54:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n>  int run_hook(const char *index_file, const char *name, ...)\n>  {\n>  \tstruct child_process hook;\n> -\tconst char **argv = NULL, *env[2];\n> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> +\tconst char *p, *env[2];\n\nGiven that in argv-array.h you define it as\n\n    #define ARGV_ARRAY_INIT { empty_argv, 0, 0 };\n\nthe above will introduce decl-after-stmt.\n"},{"id":"175497","messageId":"20110914185658.GA21352@sigill.intra.peff.net","threadId":"28384","inReplyTo":"7vty8ft0oc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] run_hook: use argv_array API","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-14T18:56:58Z","receivedAt":"2011-09-14T18:56:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 14, 2011 at 11:54:11AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n> >  int run_hook(const char *index_file, const char *name, ...)\n> >  {\n> >  \tstruct child_process hook;\n> > -\tconst char **argv = NULL, *env[2];\n> > +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n> > +\tconst char *p, *env[2];\n> \n> Given that in argv-array.h you define it as\n> \n>     #define ARGV_ARRAY_INIT { empty_argv, 0, 0 };\n> \n> the above will introduce decl-after-stmt.\n\nOops. Can you squash in removing the semicolon from the macro\ndefinition?\n\n-Peff\n"},{"id":"175498","messageId":"7vpqj3t0cz.fsf@alter.siamese.dyndns.org","threadId":"28384","inReplyTo":"20110913220421.GA24549@sigill.intra.peff.net","subject":"Re: [nit] diff func headers ignore context","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T19:01:00Z","receivedAt":"2011-09-14T19:01:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:\n>\n>> @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n>>  int run_hook(const char *index_file, const char *name, ...)\n>>  {\n>>  \tstruct child_process hook;\n>> -\tconst char **argv = NULL, *env[2];\n>> +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n>\n> I find this diff function header pretty confusing. Of course we're not\n> in finish_async, as you can see by the fact that the context contains\n> the start of run_hook.\n>\n> I don't think this is something that can be solved with xfuncname\n> config; we would have to teach xdiff to look at context lines when\n> picking a header line.\n>\n> Am I the only one who finds this confusing? Can anyone think of a reason\n> to keep showing finish_async in this example?\n\nWould this be sufficient?  Instead of looking for the first line that\nmatches the \"beginning\" pattern going backwards starting from one line\nbefore the displayed context, we start our examination at the first line\nshown in the context.\n\n xdiff/xemit.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 277e2ee..5f9c0e0 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -131,7 +131,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\tif (xecfg->flags & XDL_EMIT_FUNCNAMES) {\n \t\t\tlong l;\n-\t\t\tfor (l = s1 - 1; l >= 0 && l > funclineprev; l--) {\n+\t\t\tfor (l = s1; l >= 0 && l > funclineprev; l--) {\n \t\t\t\tconst char *rec;\n \t\t\t\tlong reclen = xdl_get_rec(&xe->xdf1, l, &rec);\n \t\t\t\tlong newfunclen = ff(rec, reclen, funcbuf,\n"},{"id":"175508","messageId":"7vd3f2uc4q.fsf@alter.siamese.dyndns.org","threadId":"28384","inReplyTo":"20110914185658.GA21352@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] run_hook: use argv_array API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-14T20:01:25Z","receivedAt":"2011-09-14T20:01:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Sep 14, 2011 at 11:54:11AM -0700, Junio C Hamano wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > @@ -609,26 +610,23 @@ int finish_async(struct async *async)\n>> >  int run_hook(const char *index_file, const char *name, ...)\n>> >  {\n>> >  \tstruct child_process hook;\n>> > -\tconst char **argv = NULL, *env[2];\n>> > +\tstruct argv_array argv = ARGV_ARRAY_INIT;\n>> > +\tconst char *p, *env[2];\n>> \n>> Given that in argv-array.h you define it as\n>> \n>>     #define ARGV_ARRAY_INIT { empty_argv, 0, 0 };\n>> \n>> the above will introduce decl-after-stmt.\n>\n> Oops. Can you squash in removing the semicolon from the macro\n> definition?\n\nYes, I did. Thanks.\n"},{"id":"175522","messageId":"20110914231804.GB5611@sigill.intra.peff.net","threadId":"28384","inReplyTo":"CAP8UFD1vxP9ABgJpM99hxDWWLeGO_QW7QLVFq1f-teu1fiCftA@mail.gmail.com","subject":"Re: [PATCH 3/7] refactor argv_array into generic code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-14T23:18:04Z","receivedAt":"2011-09-14T23:18:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 14, 2011 at 07:54:48AM +0200, Christian Couder wrote:\n\n> In sha1-array you called the \"push\" function \"sha1_array_append\"\n> instead of \"sha1_array_push\", so I wonder why here you call them\n> \"*_push*\" instead of \"*_append*\"?\n\nI dunno. It just seemed natural to write \"push\" in the context of argv.\nMaybe too much perl (push, pop, shift, unshift).\n\nargv_array_append does make sense. One could argue that\nsha1_array_append actually doesn't. True, it does append to the end of\nthe array, but after writing the docs for it yesterday, I realized that\nit less of an array, and more of a set container. Because the point of\nusing it is the optimized lookup/unique function, which is going to sort\nit. The array is really just an implementation detail.\n\nSo arguably it should be \"struct sha1_set\", and \"sha1_set_insert\" or\nsomething. I'm not sure if it's really worth changing (because this is\nC, our data structures tend to be a little leaky, anyway, and you _can_\nuse sha1_array as an ordered list if you want; just don't call the\nlookup or sorting functions).\n\n-Peff\n"}]}