{"thread":{"id":"5334","subject":"[PATCH] Helper function to shell quote all arg values at once.","startedAt":"2006-08-20T06:07:49Z","lastAt":"2006-08-21T05:15:28Z","messageCount":4,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"25637","messageId":"20060820080749.e31276dd.chriscool@tuxfamily.org","threadId":"5334","inReplyTo":null,"subject":"[PATCH] Helper function to shell quote all arg values at once.","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2006-08-20T06:07:49Z","receivedAt":"2006-08-20T06:07:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The new sq_quote_argv function is used to refactor the\ntracing code in \"git.c\" and \"exec_cmd.c\".\nThis function allocates memory and fills it with a string\ncontaining the quoted argument values. Then it returns a\npointer to this memory that must be freed afterwards.\n---\n exec_cmd.c |   11 +++--------\n git.c      |   22 +++++++---------------\n quote.c    |   33 +++++++++++++++++++++++++++++++++\n quote.h    |    1 +\n 4 files changed, 44 insertions(+), 23 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex e30936d..6d215d8 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -98,15 +98,10 @@ int execv_git_cmd(const char **argv)\n \t\targv[0] = git_command;\n \n \t\tif (getenv(\"GIT_TRACE\")) {\n-\t\t\tconst char **p = argv;\n-\t\t\tfputs(\"trace: exec:\", stderr);\n-\t\t\twhile (*p) {\n-\t\t\t\tfputc(' ', stderr);\n-\t\t\t\tsq_quote_print(stderr, *p);\n-\t\t\t\t++p;\n-\t\t\t}\n-\t\t\tputc('\\n', stderr);\n+\t\t\tchar *arg_str = sq_quote_argv(argv, -1);\n+\t\t\tfprintf(stderr, \"trace: exec: %s\\n\", arg_str);\n \t\t\tfflush(stderr);\n+\t\t\tfree(arg_str);\n \t\t}\n \n \t\t/* execve() can only ever return if it fails */\ndiff --git a/git.c b/git.c\nindex 930998b..ff4ba66 100644\n--- a/git.c\n+++ b/git.c\n@@ -180,15 +180,11 @@ static int handle_alias(int *argcp, cons\n \t\t\tdie(\"recursive alias: %s\", alias_command);\n \n \t\tif (getenv(\"GIT_TRACE\")) {\n-\t\t\tint i;\n-\t\t\tfprintf(stderr, \"trace: alias expansion: %s =>\",\n-\t\t\t\talias_command);\n-\t\t\tfor (i = 0; i < count; ++i) {\n-\t\t\t\tfputc(' ', stderr);\n-\t\t\t\tsq_quote_print(stderr, new_argv[i]);\n-\t\t\t}\n-\t\t\tfputc('\\n', stderr);\n+\t\t\tchar *arg_str = sq_quote_argv(new_argv, count);\n+\t\t\tfprintf(stderr, \"trace: alias expansion: %s => %s\\n\",\n+\t\t\t\talias_command, arg_str);\n \t\t\tfflush(stderr);\n+\t\t\tfree(arg_str);\n \t\t}\n \n \t\tnew_argv = realloc(new_argv, sizeof(char*) *\n@@ -292,14 +288,10 @@ static void handle_internal_command(int \n \t\tif (p->option & USE_PAGER)\n \t\t\tsetup_pager();\n \t\tif (getenv(\"GIT_TRACE\")) {\n-\t\t\tint i;\n-\t\t\tfprintf(stderr, \"trace: built-in: git\");\n-\t\t\tfor (i = 0; i < argc; ++i) {\n-\t\t\t\tfputc(' ', stderr);\n-\t\t\t\tsq_quote_print(stderr, argv[i]);\n-\t\t\t}\n-\t\t\tputc('\\n', stderr);\n+\t\t\tchar *arg_str = sq_quote_argv(argv, argc);\n+\t\t\tfprintf(stderr, \"trace: built-in: git %s\\n\", arg_str);\n \t\t\tfflush(stderr);\n+\t\t\tfree(arg_str);\n \t\t}\n \n \t\texit(p->fn(argc, argv, prefix));\ndiff --git a/quote.c b/quote.c\nindex e220dcc..2e6289b 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -74,6 +74,39 @@ char *sq_quote(const char *src)\n \treturn buf;\n }\n \n+char *sq_quote_argv(const char** argv, int count)\n+{\n+\tchar *buf, *to;\n+\tint i;\n+\tsize_t len;\n+\n+\t/* Count argv if needed. */\n+\tif (count < 0) {\n+\t\tchar **p = (char **)argv;\n+\t\tcount = 0;\n+\t\twhile (*p++) count++;\n+\t}\n+\n+\t/* Get destination buffer length. */\n+\tlen = count ? count : 1;\n+\tfor (i = 0; i < count; ++i)\n+\t\tlen += sq_quote_buf(NULL, 0, argv[i]);\n+\n+\t/* Alloc destination buffer. */\n+\tto = buf = xmalloc(len);\n+\n+\t/* Copy into destination buffer. */\n+\tfor (i = 0; i < count; ++i) {\n+\t\tif (i) *to++ = ' ';\n+\t\tto += sq_quote_buf(to, len, argv[i]);\n+\t}\n+\n+\tif (!count)\n+\t\t*buf = 0;\n+\t\n+\treturn buf;\n+}\n+\n char *sq_dequote(char *arg)\n {\n \tchar *dst = arg;\ndiff --git a/quote.h b/quote.h\nindex fc5481e..a6c4611 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -31,6 +31,7 @@ #include <stdio.h>\n extern char *sq_quote(const char *src);\n extern void sq_quote_print(FILE *stream, const char *src);\n extern size_t sq_quote_buf(char *dst, size_t n, const char *src);\n+extern char *sq_quote_argv(const char** argv, int count);\n \n /* This unwraps what sq_quote() produces in place, but returns\n  * NULL if the input does not look like what sq_quote would have\n-- \n1.4.2.g44496-dirty\n"},{"id":"25653","messageId":"200608202032.40737.chriscool@tuxfamily.org","threadId":"5334","inReplyTo":"20060820080749.e31276dd.chriscool@tuxfamily.org","subject":"Re: [PATCH] Helper function to shell quote all arg values at once.","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2006-08-20T18:32:40Z","receivedAt":"2006-08-20T18:32:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"I forgot to sign this patch off :\n\n> The new sq_quote_argv function is used to refactor the\n> tracing code in \"git.c\" and \"exec_cmd.c\".\n> This function allocates memory and fills it with a string\n> containing the quoted argument values. Then it returns a\n> pointer to this memory that must be freed afterwards.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n\nChristian.\n"},{"id":"25662","messageId":"7vsljrxbg8.fsf@assigned-by-dhcp.cox.net","threadId":"5334","inReplyTo":"20060820080749.e31276dd.chriscool@tuxfamily.org","subject":"Re: [PATCH] Helper function to shell quote all arg values at once.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-20T22:57:11Z","receivedAt":"2006-08-20T22:57:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> The new sq_quote_argv function is used to refactor the\n> tracing code in \"git.c\" and \"exec_cmd.c\".\n> This function allocates memory and fills it with a string\n> containing the quoted argument values. Then it returns a\n> pointer to this memory that must be freed afterwards.\n\nSorry, I do not see a point in this.\n\nIf your original were doing malloc-print-free per iteration,\nthen perhaps it makes sense to first format all into one\nallocated buffer, print all, and then free at once, like this\npatch does.  But that was not what the original had.\n\nIf the new function were to get a (const char **) array and\nFILE *, and print them, quoted and separated with spaces, then\nit would have shortened what two call sites did, which would\nhave been an improvement.  But that is not what this patch does,\neither.\n"},{"id":"25672","messageId":"200608210715.28585.chriscool@tuxfamily.org","threadId":"5334","inReplyTo":"7vsljrxbg8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Helper function to shell quote all arg values at once.","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2006-08-21T05:15:28Z","receivedAt":"2006-08-21T05:15:28Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 21 août 2006 00:57, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> \n> > The new sq_quote_argv function is used to refactor the\n> > tracing code in \"git.c\" and \"exec_cmd.c\".\n> > This function allocates memory and fills it with a string\n> > containing the quoted argument values. Then it returns a\n> > pointer to this memory that must be freed afterwards.\n> \n> Sorry, I do not see a point in this.\n> \n> If your original were doing malloc-print-free per iteration,\n> then perhaps it makes sense to first format all into one\n> allocated buffer, print all, and then free at once, like this\n> patch does.  But that was not what the original had.\n\nI thought that malloc-print-free was Ok, since there is already sq_quote\nin quote.c that does it and is used in connect.c (except perhaps that the result from sq_quote is not freed).\n\n> If the new function were to get a (const char **) array and\n> FILE *, and print them, quoted and separated with spaces, then\n> it would have shortened what two call sites did, which would\n> have been an improvement.  But that is not what this patch does,\n> either.\n\nThe patch still shortens the 3 calls site in git.c (-8 lines) and exec_cmd.c (-5 lines).\nBut anyway I will rewrite it so that the new function takes (FILE*, const char **, int count) as argument.\n\nOr perhaps I need only make this new function call sq_quote_argv ? So I only need to send another patch on top of this one ?\n\nThanks,\nChristian.\n"}]}