{"thread":{"id":"9330","subject":"[PATCH] Allow git_author|committer_info be called in the same expression","startedAt":"2007-07-31T16:57:05Z","lastAt":"2007-08-01T09:35:45Z","messageCount":3,"participants":["Alex Riesen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"49236","messageId":"81b0412b0707310957x62ac9d28j60104bffb46a80b7@mail.gmail.com","threadId":"9330","inReplyTo":null,"subject":"[PATCH] Allow git_author|committer_info be called in the same expression","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-07-31T16:57:05Z","receivedAt":"2007-07-31T16:57:05Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Signed-off-by: Alex Riesen \n---\n ident.c | 49 ++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 32 insertions(+), 17 deletions(-)\n\nOn 7/31/07, Kristian Høgsberg wrote:\n> There's number of buffers that don't get freed: the strbuf, the commit\n> message buffer, and the strdup'ed author and committer info. All the\n> leaks are not critical since the process exits immediately. As for the\n> strbuf leak, I was thinking about renaming strbuf_begin to strbuf_reset\n> and making it public[1], which will then be used for freeing up strbuf\n> memory. The message buffer leak should be fixed by adding a\n> strbuf_read_fd() that just reads it straight into the strbuf. The\n> xstrdup's are necessary because fmt_ident uses a static buffer (thanks,\n> test case :). We could add rotating static buffers for fmt_ident like\n> git_path and avoid the strdups, but again, the leaks are not critical.\n\nAch, that's why... Junio, how about this then? I'd even suggest adding\nthe buffer argument to fmt_ident and use it everywhere (ATM there is\nonly one user outside of ident.c: builtin-blame.c).\nThis one is applicable immediately, though\n\n\nFrom 0c86739aa9f5db19ebcd2bc041622c317611c87f Mon Sep 17 00:00:00 2001\nFrom: Alex Riesen \nDate: Tue, 31 Jul 2007 18:48:23 +0200\nSubject: [PATCH] Allow git_author|committer_info be called in the same expression\n\n\nSigned-off-by: Alex Riesen \n---\n ident.c | 49 ++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 32 insertions(+), 17 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 6612d17..0d59090 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -192,10 +192,10 @@ static const char *env_hint =\n \"Add --global to set your account\\'s default\\n\"\n \"\\n\";\n \n-const char *fmt_ident(const char *name, const char *email,\n-\t\t const char *date_str, int error_on_no_name)\n+#define FMT_IDENT_BUFSIZE 1000\n+static char *fmt_ident_buf(char *buffer, const char *name, const char *email,\n+\t\t\t const char *date_str, int error_on_no_name)\n {\n-\tstatic char buffer[1000];\n \tchar date[50];\n \tint i;\n \n@@ -227,29 +227,44 @@ const char *fmt_ident(const char *name, const char *email,\n \tif (date_str)\n \t\tparse_date(date_str, date, sizeof(date));\n \n-\ti = copy(buffer, sizeof(buffer), 0, name);\n-\ti = add_raw(buffer, sizeof(buffer), i, \" <\");\n-\ti = copy(buffer, sizeof(buffer), i, email);\n-\ti = add_raw(buffer, sizeof(buffer), i, \"> \");\n-\ti = copy(buffer, sizeof(buffer), i, date);\n-\tif (i >= sizeof(buffer))\n+\ti = copy(buffer, FMT_IDENT_BUFSIZE, 0, name);\n+\ti = add_raw(buffer, FMT_IDENT_BUFSIZE, i, \" <\");\n+\ti = copy(buffer, FMT_IDENT_BUFSIZE, i, email);\n+\ti = add_raw(buffer, FMT_IDENT_BUFSIZE, i, \"> \");\n+\ti = copy(buffer, FMT_IDENT_BUFSIZE, i, date);\n+\tif (i >= FMT_IDENT_BUFSIZE)\n \t\tdie(\"Impossibly long personal identifier\");\n \tbuffer[i] = 0;\n \treturn buffer;\n }\n \n+const char *fmt_ident(const char *name, const char *email,\n+\t\t const char *date_str, int error_on_no_name)\n+{\n+\tstatic char buffer[FMT_IDENT_BUFSIZE];\n+\treturn fmt_ident_buf(buffer, name, email, date_str, error_on_no_name);\n+}\n+\n const char *git_author_info(int error_on_no_name)\n {\n-\treturn fmt_ident(getenv(\"GIT_AUTHOR_NAME\"),\n-\t\t\t getenv(\"GIT_AUTHOR_EMAIL\"),\n-\t\t\t getenv(\"GIT_AUTHOR_DATE\"),\n-\t\t\t error_on_no_name);\n+\tstatic char *buffer;\n+\tif (!buffer)\n+\t\tbuffer = xmalloc(FMT_IDENT_BUFSIZE);\n+\treturn fmt_ident_buf(buffer,\n+\t\t\t getenv(\"GIT_AUTHOR_NAME\"),\n+\t\t\t getenv(\"GIT_AUTHOR_EMAIL\"),\n+\t\t\t getenv(\"GIT_AUTHOR_DATE\"),\n+\t\t\t error_on_no_name);\n }\n \n const char *git_committer_info(int error_on_no_name)\n {\n-\treturn fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n-\t\t\t getenv(\"GIT_COMMITTER_DATE\"),\n-\t\t\t error_on_no_name);\n+\tstatic char *buffer;\n+\tif (!buffer)\n+\t\tbuffer = xmalloc(FMT_IDENT_BUFSIZE);\n+\treturn fmt_ident_buf(buffer,\n+\t\t\t getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n+\t\t\t getenv(\"GIT_COMMITTER_DATE\"),\n+\t\t\t error_on_no_name);\n }\n-- \n1.5.3.rc3.132.g39179\n\n"},{"id":"49294","messageId":"7vhcnj7gkt.fsf@assigned-by-dhcp.cox.net","threadId":"9330","inReplyTo":"81b0412b0707310957x62ac9d28j60104bffb46a80b7@mail.gmail.com","subject":"Re: [PATCH] Allow git_author|committer_info be called in the same expression","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-08-01T09:24:34Z","receivedAt":"2007-08-01T09:24:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Riesen\" writes:\n\n> const char *git_committer_info(int error_on_no_name)\n> {\n> -\treturn fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n> -\t\t\t getenv(\"GIT_COMMITTER_EMAIL\"),\n> -\t\t\t getenv(\"GIT_COMMITTER_DATE\"),\n> -\t\t\t error_on_no_name);\n> +\tstatic char *buffer;\n> +\tif (!buffer)\n> +\t\tbuffer = xmalloc(FMT_IDENT_BUFSIZE);\n\nIt is not like we are aiming to run on embedded devices, I think\ntrying to save static memory when the function is not called\nby doing things like this is not worth it. Why not just:\n\n\tstatic char buffer[FMT_IDENT_BUFSIZE];\n\ninstead?\n\nIn any case, I'd really like to slow things down and ask\neverybody to concentrate on finding and fixing bugs in what are\nscheduled for 1.5.3 for a week or so.\n\nThat means the current \"master\".\n\nI did not merge Johannes's work tree rationalization patch to\n\"master\" yet (it is now part of \"next\"), but I would like to\nhave it in, as it makes the code much easier to read compared to\nthe work-tree changes currently in \"master\". Even though the\none currently in \"master\" does not seem to regress the normal\nusage of not using GIT_WORK_TREE environment, we would be better\noff if a new feature is released in a shape that is maintainable.\n\nOther than that topic, everything on \"next\" will be post 1.5.3.\n\nPlease.\n\nI'd like to do -rc4 this weekend, and then 1.5.3 final by mid\nAugust at the latest, but hopefully sooner.\n\nAs to parts of the system that other people are primarily in\ncharge of, git-gui 0.8.0 is already in as promised, updates to\ngitk were merged, and we've had a nice set of improvements from\nJakub on gitweb already. Bruce seems to have a beginning of the\nuser manual reorganization, but I am getting the feeling it is\nnot ready yet.\n"},{"id":"49296","messageId":"81b0412b0708010235j3c731b38v32b5a717ae9f1f0b@mail.gmail.com","threadId":"9330","inReplyTo":"7vhcnj7gkt.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Allow git_author|committer_info be called in the same expression","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-08-01T09:35:45Z","receivedAt":"2007-08-01T09:35:45Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 8/1/07, Junio C Hamano wrote:\n> \"Alex Riesen\" writes:\n>\n> > const char *git_committer_info(int error_on_no_name)\n> > {\n> > - return fmt_ident(getenv(\"GIT_COMMITTER_NAME\"),\n> > - getenv(\"GIT_COMMITTER_EMAIL\"),\n> > - getenv(\"GIT_COMMITTER_DATE\"),\n> > - error_on_no_name);\n> > + static char *buffer;\n> > + if (!buffer)\n> > + buffer = xmalloc(FMT_IDENT_BUFSIZE);\n>\n> It is not like we are aiming to run on embedded devices, I think\n\nActually, I am planning to run it on Neo1973 (Openmoko),\nwhich is quite embedded with its 128mb.\n\n> trying to save static memory when the function is not called\n> by doing things like this is not worth it. Why not just:\n>\n> static char buffer[FMT_IDENT_BUFSIZE];\n>\n> instead?\n\nJust a custom. This is a big bugger, err... buffer.\n\n> As to parts of the system that other people are primarily in\n> charge of, git-gui 0.8.0 is already in as promised, updates to\n> gitk were merged, and we've had a nice set of improvements from\n\nBTW, gitk. Have you seen the gitk patches regarding errors in non-GIT\nand not-yet-GIT directory?\n"}]}