{"thread":{"id":"18257","subject":"[PATCH] Removed unnecessary use of global variables.","startedAt":"2009-03-11T00:09:28Z","lastAt":"2009-03-11T22:00:48Z","messageCount":6,"participants":["Erik Faye-Lund","Johannes Schindelin","Nanako Shiraishi","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107625","messageId":"1236730168-7164-1-git-send-email-kusmabite@gmail.com","threadId":"18257","inReplyTo":null,"subject":"[PATCH] Removed unnecessary use of global variables.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-11T00:09:28Z","receivedAt":"2009-03-11T00:09:28Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"git_config() now takes a third data-parameter that is passed back\nto the callback-function. At the time this code was written, that\nparameter did not exist, so a somewhat nasty (but by all means\ncorrect) use of global variables was introduced. In commit\nef90d6d4208a5130185b04f06e5f90a5f9959fe3 Johannes Schindelin\n introduced a parameter for similar\npurposes.\n\nI've changed the code to utilize this parameter to pass the\nstring. In addition, I've made the function calculate the string\nlength on usage instead, to reduce the parameters needed to what\nthe callback-interface supplies.\n\nSigned-off-by: Erik Faye-Lund \n---\n connect.c | 16 ++++++----------\n 1 files changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 2f23ab3..98fbaea 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -371,14 +371,13 @@ static void git_tcp_connect(int fd[2], char *host, int flags)\n \tfd[1] = dup(sockfd);\n }\n \n-\n static char *git_proxy_command;\n-static const char *rhost_name;\n-static int rhost_len;\n-\n static int git_proxy_command_options(const char *var, const char *value,\n-\t\tvoid *cb)\n+\t\tvoid *data)\n {\n+\tconst char *rhost_name = data;\n+\tconst size_t rhost_len = strlen(rhost_name);\n+\n \tif (!strcmp(var, \"core.gitproxy\")) {\n \t\tconst char *for_pos;\n \t\tint matchlen = -1;\n@@ -421,16 +420,13 @@ static int git_proxy_command_options(const char *var, const char *value,\n \t\treturn 0;\n \t}\n \n-\treturn git_default_config(var, value, cb);\n+\treturn git_default_config(var, value, data);\n }\n \n static int git_use_proxy(const char *host)\n {\n-\trhost_name = host;\n-\trhost_len = strlen(host);\n \tgit_proxy_command = getenv(\"GIT_PROXY_COMMAND\");\n-\tgit_config(git_proxy_command_options, NULL);\n-\trhost_name = NULL;\n+\tgit_config(git_proxy_command_options, (void*)host);\n \treturn (git_proxy_command && *git_proxy_command);\n }\n \n-- \n1.6.2.GIT\n"},{"id":"107626","messageId":"55ef48ab-403d-41b8-bf16-203bb0f73fc7@b16g2000yqb.googlegroups.com","threadId":"18257","inReplyTo":"1236730168-7164-1-git-send-email-kusmabite@gmail.com","subject":"Re: Removed unnecessary use of global variables.","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-03-11T01:19:46Z","receivedAt":"2009-03-11T01:19:46Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Sorry about the whole double-post thing, this whole submitting patches\nover email thing is new to me, and I'm making too many mistakes right\nnow. Hopefully, I'll improve in the future ;)\n\nOn Mar 11, 1:09 am, Erik Faye-Lund wrote:\n> git_config() now takes a third data-parameter that is passed back\n> to the callback-function. At the time this code was written, that\n> parameter did not exist, so a somewhat nasty (but by all means\n> correct) use of global variables was introduced. In commit\n> ef90d6d4208a5130185b04f06e5f90a5f9959fe3 Johannes Schindelin\n> introduced a parameter for similar\n> purposes.\n>\n> I've changed the code to utilize this parameter to pass the\n> string. In addition, I've made the function calculate the string\n> length on usage instead, to reduce the parameters needed to what\n> the callback-interface supplies.\n>\n> Signed-off-by: Erik Faye-Lund \n> ---\n>  connect.c |   16 ++++++----------\n>  1 files changed, 6 insertions(+), 10 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index 2f23ab3..98fbaea 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -371,14 +371,13 @@ static void git_tcp_connect(int fd[2], char *host, int flags)\n>         fd[1] = dup(sockfd);\n>  }\n>\n> -\n>  static char *git_proxy_command;\n> -static const char *rhost_name;\n> -static int rhost_len;\n> -\n>  static int git_proxy_command_options(const char *var, const char *value,\n> -               void *cb)\n> +               void *data)\n>  {\n> +       const char *rhost_name = data;\n> +       const size_t rhost_len = strlen(rhost_name);\n> +\n>         if (!strcmp(var, \"core.gitproxy\")) {\n>                 const char *for_pos;\n>                 int matchlen = -1;\n> @@ -421,16 +420,13 @@ static int git_proxy_command_options(const char *var, const char *value,\n>                 return 0;\n>         }\n>\n> -       return git_default_config(var, value, cb);\n> +       return git_default_config(var, value, data);\n>  }\n>\n>  static int git_use_proxy(const char *host)\n>  {\n> -       rhost_name = host;\n> -       rhost_len = strlen(host);\n>         git_proxy_command = getenv(\"GIT_PROXY_COMMAND\");\n> -       git_config(git_proxy_command_options, NULL);\n> -       rhost_name = NULL;\n> +       git_config(git_proxy_command_options, (void*)host);\n>         return (git_proxy_command && *git_proxy_command);\n>  }\n>\n> --\n> 1.6.2.GIT\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majord...@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"107667","messageId":"alpine.DEB.1.00.0903111126280.10279@pacific.mpi-cbg.de","threadId":"18257","inReplyTo":"1236730168-7164-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH] Removed unnecessary use of global variables.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-11T10:30:07Z","receivedAt":"2009-03-11T10:30:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 11 Mar 2009, Erik Faye-Lund wrote:\n\n> git_config() now takes a third data-parameter that is passed back\n> to the callback-function. At the time this code was written, that\n> parameter did not exist, so a somewhat nasty (but by all means\n> correct) use of global variables was introduced. In commit\n> ef90d6d4208a5130185b04f06e5f90a5f9959fe3 Johannes Schindelin\n> introduced a parameter for similar\n> purposes.\n\nWe tend to quote commits in this form: ef90d6d(Provide git_config with a \ncallback-data parameter)\n\n> I've changed the code to utilize this parameter to pass the\n> string. In addition, I've made the function calculate the string\n> length on usage instead, to reduce the parameters needed to what\n> the callback-interface supplies.\n\nUsually, commit messages are held in a more imperative form than a \nsubjective one:\n\n\tUtilize this parameter to pass the string.\n\n> diff --git a/connect.c b/connect.c\n> index 2f23ab3..98fbaea 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -371,14 +371,13 @@ static void git_tcp_connect(int fd[2], char *host, int flags)\n> \tfd[1] = dup(sockfd);\n> }\n> \n> -\n> static char *git_proxy_command;\n> -static const char *rhost_name;\n> -static int rhost_len;\n> -\n> static int git_proxy_command_options(const char *var, const char *value,\n> -\t\tvoid *cb)\n> +\t\tvoid *data)\n> {\n> +\tconst char *rhost_name = data;\n> +\tconst size_t rhost_len = strlen(rhost_name);\n> +\n\ngit_proxy_command_options is called for each and every config variable. \nThe idea of having the length in a local variable was to avoid \nrecalculating the length each and every time. I think I'd actually use a \nstrbuf for that.\n\nBTW I would not rename \"cb\", as it is only distracting the reader of the \npatch.\n\nBut I like the idea of your patch (as you can see from me replying ;-)\n\nThanks,\nDscho\n"},{"id":"107668","messageId":"alpine.DEB.1.00.0903111130460.10279@pacific.mpi-cbg.de","threadId":"18257","inReplyTo":"55ef48ab-403d-41b8-bf16-203bb0f73fc7@b16g2000yqb.googlegroups.com","subject":"Re: Removed unnecessary use of global variables.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-11T10:31:04Z","receivedAt":"2009-03-11T10:31:04Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Erik Faye-Lund wrote:\n\n> Sorry about the whole double-post thing, this whole submitting patches \n> over email thing is new to me, and I'm making too many mistakes right \n> now. Hopefully, I'll improve in the future ;)\n\nNo need to be sorry. Thanks for your contribution!\n\nCiao,\nDscho\n"},{"id":"107739","messageId":"20090312064237.6117@nanako3.lavabit.com","threadId":"18257","inReplyTo":"alpine.DEB.1.00.0903111126280.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] Removed unnecessary use of global variables.","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-03-11T21:42:37Z","receivedAt":"2009-03-11T21:42:37Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Johannes Schindelin :\n\n> Hi,\n>\n> On Wed, 11 Mar 2009, Erik Faye-Lund wrote:\n>\n>> git_config() now takes a third data-parameter that is passed back\n>> to the callback-function. At the time this code was written, that\n>> parameter did not exist, so a somewhat nasty (but by all means\n>> correct) use of global variables was introduced. In commit\n>> ef90d6d4208a5130185b04f06e5f90a5f9959fe3 Johannes Schindelin\n>> introduced a parameter for similar\n>> purposes.\n>\n> We tend to quote commits in this form: ef90d6d(Provide git_config with a \n> callback-data parameter)\n\nYour review comments are a subset of the ones Junio sent about 8 hours before you did, and are almost identical except for the comment on the subject line yours didn't have. I'm curious about two things.\n\n1. Are you and Junio one same person, and if so what made you change your mind during these 8 hours ;-)?\n\n2. Junio said \"ef90d6d (Provide git_config with a callback-data parameter, 2008-05-14)\" and yours is slightly different. Both are equally readable but I think it would help to make sure everybody uses the same format within one project. Do we need a helper command that everybody can use?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"107741","messageId":"7vab7r3ejz.fsf@gitster.siamese.dyndns.org","threadId":"18257","inReplyTo":"20090312064237.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Removed unnecessary use of global variables.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-11T22:00:48Z","receivedAt":"2009-03-11T22:00:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> 1. Are you and Junio one same person, and if so what made you change your mind during these 8 hours ;-)?\n\nLast year during Gittogether, Dscho and I were seen in a room sitting\nparhaps 4 meters apart; I suspect we are different people.\n\nhttp://picasaweb.google.com/dsymonds/GitTogether08#5263025230667494002\n\n> 2. Junio said \"ef90d6d (Provide git_config with a callback-data parameter, 2008-05-14)\" and yours is slightly different. Both are equally readable but I think it would help to make sure everybody uses the same format within one project. Do we need a helper command that everybody can use?\n\nI use these two aliases while I am writing e-mails quite often:\n\n[alias]\n\tone = \"!sh -c 'git show -s --pretty=\\\"format:%h (%s, %ai\\\" \\\"$@\\\" | sed -e \\\"s/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9][0-9][0-9]$/)/\\\"' -\"\n\twho = \"!sh -c 'git log -1 --pretty=\\\"format:%an <%ae>\\\" --author=\\\"$1\\\"' -\"\n"}]}