{"thread":{"id":"18258","subject":"[PATCH v2] connect.c: remove a few globals by using git_config callback data","startedAt":"2009-03-11T02:38:12Z","lastAt":"2009-03-11T13:28:24Z","messageCount":4,"participants":["Erik Faye-Lund","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"107631","messageId":"1236739092-8280-1-git-send-email-kusmabite@gmail.com","threadId":"18258","inReplyTo":null,"subject":"[PATCH v2] connect.c: remove a few globals by using git_config callback data","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2009-03-11T02:38:12Z","receivedAt":"2009-03-11T02:38:12Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Since ef90d6d (Provide git_config with a callback-data parameter,\n2008-05-14), git_config() takes a callback data pointer that can be\nused to pass extra parameters to the parsing function. The codepath\nto parse configuration variables related to git proxy predates this\nfacility and used a pair of file scope static variables instead.\n\nThis patch removes the need for these global variables by passing the\nname of the host we are trying to access as the callback data.\n\nSigned-off-by: Erik Faye-Lund \n---\n connect.c | 9 +++------\n 1 files changed, 3 insertions(+), 6 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 2f23ab3..0a35cc1 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -373,8 +373,6 @@ static void git_tcp_connect(int fd[2], char *host, int flags)\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@@ -383,6 +381,8 @@ static int git_proxy_command_options(const char *var, const char *value,\n \t\tconst char *for_pos;\n \t\tint matchlen = -1;\n \t\tint hostlen;\n+\t\tconst char *rhost_name = cb;\n+\t\tint rhost_len = strlen(rhost_name);\n \n \t\tif (git_proxy_command)\n \t\t\treturn 0;\n@@ -426,11 +426,8 @@ static int git_proxy_command_options(const char *var, const char *value,\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":"107672","messageId":"alpine.DEB.1.00.0903111251160.10279@pacific.mpi-cbg.de","threadId":"18258","inReplyTo":"1236739092-8280-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH v2] connect.c: remove a few globals by using git_config callback data","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-11T11:52:41Z","receivedAt":"2009-03-11T11:52:41Z","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> Since ef90d6d (Provide git_config with a callback-data parameter,\n> 2008-05-14), git_config() takes a callback data pointer that can be\n> used to pass extra parameters to the parsing function. The codepath\n> to parse configuration variables related to git proxy predates this\n> facility and used a pair of file scope static variables instead.\n> \n> This patch removes the need for these global variables by passing the\n> name of the host we are trying to access as the callback data.\n> \n> Signed-off-by: Erik Faye-Lund \n\nThanks!\n\n> @@ -383,6 +381,8 @@ static int git_proxy_command_options(const char *var, const char *value,\n> \t\tconst char *for_pos;\n> \t\tint matchlen = -1;\n> \t\tint hostlen;\n> +\t\tconst char *rhost_name = cb;\n> +\t\tint rhost_len = strlen(rhost_name);\n\nI see that you still calculate the length everytime \ngit_proxy_command_options() is called -- which is for every config \nvariable.\n\n> @@ -426,11 +426,8 @@ static int git_proxy_command_options(const char *var, const char *value,\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\nThe (void *) should not be needed.\n\nThanks,\nDscho\n"},{"id":"107687","messageId":"40aa078e0903110617g60e0d7d0x1e0d94e44ef89f40@mail.gmail.com","threadId":"18258","inReplyTo":"alpine.DEB.1.00.0903111251160.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] connect.c: remove a few globals by using git_config callback data","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-03-11T13:17:53Z","receivedAt":"2009-03-11T13:17:53Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"Thanks for the feedback.\n\nOn Wed, Mar 11, 2009 at 12:52 PM, Johannes Schindelin\n wrote:\n>> @@ -383,6 +381,8 @@ static int git_proxy_command_options(const char *var, const char *value,\n>>               const char *for_pos;\n>>               int matchlen = -1;\n>>               int hostlen;\n>> +             const char *rhost_name = cb;\n>> +             int rhost_len = strlen(rhost_name);\n>\n> I see that you still calculate the length everytime\n> git_proxy_command_options() is called -- which is for every config\n> variable.\n\nActually, the length-calculation is now moved inside the check for\n\"core.gitproxy\", so it should only trigger for each config variable\nthat is \"core.gitproxy\". Hopefully, this should usually be just once\n;)\n\nThis change was suggested as a compromise by Junio.\n\n>\n>> +     git_config(git_proxy_command_options, (void*)host);\n>\n> The (void *) should not be needed.\n\nActually, it is - it casts away the const-ness of the string. Removing\nthe cast generates a warning on my installation of GCC. Perhaps I\nshould have made that clearer by casting to \"char*\" instead of\n\"void*\"?\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"107689","messageId":"alpine.DEB.1.00.0903111427120.10498@intel-tinevez-2-302","threadId":"18258","inReplyTo":"40aa078e0903110617g60e0d7d0x1e0d94e44ef89f40@mail.gmail.com","subject":"Re: [PATCH v2] connect.c: remove a few globals by using git_config callback data","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-11T13:28:24Z","receivedAt":"2009-03-11T13:28:24Z","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> On Wed, Mar 11, 2009 at 12:52 PM, Johannes Schindelin\n> wrote:\n> >> @@ -383,6 +381,8 @@ static int git_proxy_command_options(const char *var, const char *value,\n> >>               const char *for_pos;\n> >>               int matchlen = -1;\n> >>               int hostlen;\n> >> +             const char *rhost_name = cb;\n> >> +             int rhost_len = strlen(rhost_name);\n> >\n> > I see that you still calculate the length everytime\n> > git_proxy_command_options() is called -- which is for every config\n> > variable.\n> \n> Actually, the length-calculation is now moved inside the check for\n> \"core.gitproxy\", so it should only trigger for each config variable\n> that is \"core.gitproxy\". Hopefully, this should usually be just once\n> ;)\n> \n> This change was suggested as a compromise by Junio.\n\nAh, okay, I am lacking time to read all emails today, so I missed that \none.\n\n> >> +     git_config(git_proxy_command_options, (void*)host);\n> >\n> > The (void *) should not be needed.\n> \n> Actually, it is - it casts away the const-ness of the string. Removing\n> the cast generates a warning on my installation of GCC. Perhaps I\n> should have made that clearer by casting to \"char*\" instead of\n> \"void*\"?\n\nOh, that's why... Thanks for the clarification! Now only a space between \nthe void and the star, and I am happy!\n\nCiao,\nDscho\n"}]}