{"thread":{"id":"3469","subject":"[PATCH] Use setenv(), fix warnings","startedAt":"2006-02-26T15:13:46Z","lastAt":"2006-02-26T20:29:59Z","messageCount":5,"participants":["Timo Hirvonen","Junio C Hamano","Jason Riedy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"16752","messageId":"20060226171346.33ad3e47.tihirvon@gmail.com","threadId":"3469","inReplyTo":null,"subject":"[PATCH] Use setenv(), fix warnings","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-02-26T15:13:46Z","receivedAt":"2006-02-26T15:13:46Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\n - Use setenv() instead of putenv()\n - Fix -Wundef -Wold-style-definition warnings\n - Make pll_free() static\n\nSigned-off-by: Timo Hirvonen \n\n---\n\n cache.h | 2 +-\n exec_cmd.c | 2 +-\n fetch-pack.c | 2 +-\n fsck-objects.c | 4 ++--\n pack-objects.c | 2 +-\n pack-redundant.c | 4 ++--\n path.c | 2 +-\n 7 files changed, 9 insertions(+), 9 deletions(-)\n\n4001390d41ffd2c816cead47c256e598bedff452\ndiff --git a/cache.h b/cache.h\nindex 5020f07..58eec00 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -10,7 +10,7 @@\n #define deflateBound(c,s) ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n #endif\n \n-#if defined(DT_UNKNOWN) && !NO_D_TYPE_IN_DIRENT\n+#if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)\n #define DTYPE(de)\t((de)->d_type)\n #else\n #undef DT_UNKNOWN\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 55af33b..b5e59a9 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -13,7 +13,7 @@ void git_set_exec_path(const char *exec_\n \n \n /* Returns the highest-priority, location to look for git programs. */\n-const char *git_exec_path()\n+const char *git_exec_path(void)\n {\n \tconst char *env;\n \ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 09738fe..535de10 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -82,7 +82,7 @@ static void mark_common(struct commit *c\n Get the next rev to send, ignoring the common.\n */\n \n-static const unsigned char* get_rev()\n+static const unsigned char* get_rev(void)\n {\n \tstruct commit *commit = NULL;\n \ndiff --git a/fsck-objects.c b/fsck-objects.c\nindex 6439d55..f9c69f5 100644\n--- a/fsck-objects.c\n+++ b/fsck-objects.c\n@@ -20,7 +20,7 @@ static int check_strict = 0;\n static int keep_cache_objects = 0; \n static unsigned char head_sha1[20];\n \n-#if NO_D_INO_IN_DIRENT\n+#ifdef NO_D_INO_IN_DIRENT\n #define SORT_DIRENT 0\n #define DIRENT_SORT_HINT(de) 0\n #else\n@@ -483,7 +483,7 @@ int main(int argc, char **argv)\n \tif (standalone && check_full)\n \t\tdie(\"Only one of --standalone or --full can be used.\");\n \tif (standalone)\n-\t\tputenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES=\");\n+\t\tsetenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES\", \"\", 1);\n \n \tfsck_head_link();\n \tfsck_object_dir(get_object_directory());\ndiff --git a/pack-objects.c b/pack-objects.c\nindex 0287449..21ee572 100644\n--- a/pack-objects.c\n+++ b/pack-objects.c\n@@ -768,7 +768,7 @@ static int sha1_sort(const struct object\n \treturn memcmp(a->sha1, b->sha1, 20);\n }\n \n-static struct object_entry **create_final_object_list()\n+static struct object_entry **create_final_object_list(void)\n {\n \tstruct object_entry **list;\n \tint i, j;\ndiff --git a/pack-redundant.c b/pack-redundant.c\nindex 1869b38..cd81f5a 100644\n--- a/pack-redundant.c\n+++ b/pack-redundant.c\n@@ -45,7 +45,7 @@ static inline void llist_item_put(struct\n \tfree_nodes = item;\n }\n \n-static inline struct llist_item *llist_item_get()\n+static inline struct llist_item *llist_item_get(void)\n {\n \tstruct llist_item *new;\n \tif ( free_nodes ) {\n@@ -275,7 +275,7 @@ static void cmp_two_packs(struct pack_li\n \t}\n }\n \n-void pll_free(struct pll *l)\n+static void pll_free(struct pll *l)\n {\n \tstruct pll *old;\n \tstruct pack_list *opl;\ndiff --git a/path.c b/path.c\nindex 334b2bd..6500a40 100644\n--- a/path.c\n+++ b/path.c\n@@ -243,7 +243,7 @@ char *enter_repo(char *path, int strict)\n \n \tif (access(\"objects\", X_OK) == 0 && access(\"refs\", X_OK) == 0 &&\n \t validate_symref(\"HEAD\") == 0) {\n-\t\tputenv(\"GIT_DIR=.\");\n+\t\tsetenv(\"GIT_DIR\", \".\", 1);\n \t\tcheck_repository_format();\n \t\treturn path;\n \t}\n-- \n1.2.1\n"},{"id":"16754","messageId":"7vmzge570u.fsf@assigned-by-dhcp.cox.net","threadId":"3469","inReplyTo":"20060226171346.33ad3e47.tihirvon@gmail.com","subject":"Re: [PATCH] Use setenv(), fix warnings","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-26T18:06:41Z","receivedAt":"2006-02-26T18:06:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Timo Hirvonen writes:\n\n> - Use setenv() instead of putenv()\n> - Fix -Wundef -Wold-style-definition warnings\n> - Make pll_free() static\n\nI think the last one makes sense, and I can see why some people\nmay prefer -Wundef but I am not sure about the first one. Care\nto defend why we should prefer setenv()? IIRC, initially we did\nnot use setenv() anywhere because certain platforms only had\nputenv().\n\n> diff --git a/fsck-objects.c b/fsck-objects.c\n> @@ -483,7 +483,7 @@ int main(int argc, char **argv)\n> \tif (standalone && check_full)\n> \t\tdie(\"Only one of --standalone or --full can be used.\");\n> \tif (standalone)\n> -\t\tputenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES=\");\n> +\t\tsetenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES\", \"\", 1);\n\nFor platforms with only putenv we did this; here, what we really\nwanted to do was unsetenv.\n"},{"id":"16755","messageId":"20060226203756.05dcfb26.tihirvon@gmail.com","threadId":"3469","inReplyTo":"7vmzge570u.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Use setenv(), fix warnings","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-02-26T18:37:56Z","receivedAt":"2006-02-26T18:37:56Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"On Sun, 26 Feb 2006 10:06:41 -0800\nJunio C Hamano wrote:\n\n> Timo Hirvonen writes:\n> \n> > - Use setenv() instead of putenv()\n> > - Fix -Wundef -Wold-style-definition warnings\n> > - Make pll_free() static\n> \n> I think the last one makes sense, and I can see why some people\n> may prefer -Wundef but I am not sure about the first one. Care\n> to defend why we should prefer setenv()? IIRC, initially we did\n> not use setenv() anywhere because certain platforms only had\n> putenv().\n\nI was confused by putenv(3) man page. I thought it wanted malloc'ed\nstrings (no 'const' in the parameter -> warning). It appears that\nstatically allocated strings are accepted but _automatic_ variables\naren't. I noticed setenv is now in compat/ so I though it was good idea\nto use it.\n\nSorry for the noise.\n\n> > diff --git a/fsck-objects.c b/fsck-objects.c\n> > @@ -483,7 +483,7 @@ int main(int argc, char **argv)\n> > \tif (standalone && check_full)\n> > \t\tdie(\"Only one of --standalone or --full can be used.\");\n> > \tif (standalone)\n> > -\t\tputenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES=\");\n> > +\t\tsetenv(\"GIT_ALTERNATE_OBJECT_DIRECTORIES\", \"\", 1);\n> \n> For platforms with only putenv we did this; here, what we really\n> wanted to do was unsetenv.\n\nputenv(3):\n \"If the argument `string` is of the form `name`, and does not\n contain an `=' character, then the variable `name` is removed from\n the environment.\"\n\nSo the variable is emptied, not removed. But usually empty environment\nvariables are treated as if they didn't exist...\n\n-- \nhttp://onion.dynserv.net/~timo/\n"},{"id":"16757","messageId":"15703.1140982710@lotus.CS.Berkeley.EDU","threadId":"3469","inReplyTo":"20060226203756.05dcfb26.tihirvon@gmail.com","subject":"Re: [PATCH] Use setenv(), fix warnings","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2006-02-26T19:38:30Z","receivedAt":"2006-02-26T19:38:30Z","isPatch":true,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And Timo Hirvonen writes:\n - It appears that statically allocated strings are accepted but \n - _automatic_ variables aren't.\n\nThe putenv()-ed string has to exist as long as the environment\ndoes. You can twiddle the environment just by twiddling the\nstring you registered. What fun!\n\n - I noticed setenv is now in compat/ so I though it was good idea\n - to use it.\n\nAll uses of putenv() originally were setenv(). I finally realized\na compat/setenv.c was better than playing whack-a-setenv on every\nrelease... (Ah, hindsight.)\n\nNote that the current compat/setenv.c _LEAKS MEMORY_ on purpose. \nBecause putenv() requires the string to stay around, we can't\never free it. I hope any library implementing bits of git\n(libgitbit?) avoids setting environment variables.\n\n - So the variable is emptied, not removed. But usually empty environment\n - variables are treated as if they didn't exist...\n\nMore specifically, git treats both \"\" and NULL as empty, or at \nleast it did last time I checked.\n\nJason\n"},{"id":"16764","messageId":"7vd5h96eyg.fsf@assigned-by-dhcp.cox.net","threadId":"3469","inReplyTo":"20060226203756.05dcfb26.tihirvon@gmail.com","subject":"Re: [PATCH] Use setenv(), fix warnings","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-26T20:29:59Z","receivedAt":"2006-02-26T20:29:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Timo Hirvonen writes:\n\n> putenv(3):\n> \"If the argument `string` is of the form `name`, and does not\n> contain an `=' character, then the variable `name` is removed from\n> the environment.\"\n>\n> So the variable is emptied, not removed. But usually empty environment\n> variables are treated as if they didn't exist...\n\nYes we were aware of that when we did it.\n"}]}