{"thread":{"id":"12150","subject":"[PATCH 1/4] Improve message-id generation flow control for format-patch","startedAt":"2008-02-17T18:35:44Z","lastAt":"2008-02-18T18:00:24Z","messageCount":4,"participants":["Daniel Barkalow","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"69014","messageId":"alpine.LNX.1.00.0802171335240.5816@iabervon.org","threadId":"12150","inReplyTo":null,"subject":"[PATCH 1/4] Improve message-id generation flow control for format-patch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-17T18:35:44Z","receivedAt":"2008-02-17T18:35:44Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n builtin-log.c |   29 ++++++++++++++---------------\n 1 files changed, 14 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 99d69f0..867cc13 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -575,16 +575,19 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n \to2->flags = flags2;\n }\n \n-static void gen_message_id(char *dest, unsigned int length, char *base)\n+static void gen_message_id(struct rev_info *info, char *base)\n {\n \tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n \tconst char *email_start = strrchr(committer, '<');\n \tconst char *email_end = strrchr(committer, '>');\n-\tif(!email_start || !email_end || email_start > email_end - 1)\n+\tstruct strbuf buf;\n+\tif (!email_start || !email_end || email_start > email_end - 1)\n \t\tdie(\"Could not extract email from committer identity.\");\n-\tsnprintf(dest, length, \"%s.%lu.git.%.*s\", base,\n-\t\t (unsigned long) time(NULL),\n-\t\t (int)(email_end - email_start - 1), email_start + 1);\n+\tstrbuf_init(&buf, 0);\n+\tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n+\t\t    (unsigned long) time(NULL),\n+\t\t    (int)(email_end - email_start - 1), email_start + 1);\n+\tinfo->message_id = strbuf_detach(&buf, NULL);\n }\n \n static const char *clean_message_id(const char *msg_id)\n@@ -625,8 +628,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tconst char *in_reply_to = NULL;\n \tstruct patch_ids ids;\n \tchar *add_signoff = NULL;\n-\tchar message_id[1024];\n-\tchar ref_message_id[1024];\n \n \tgit_config(git_format_config);\n \tinit_revisions(&rev, prefix);\n@@ -809,15 +810,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\trev.nr = total - nr + (start_number - 1);\n \t\t/* Make the second and subsequent mails replies to the first */\n \t\tif (thread) {\n-\t\t\tif (nr == (total - 2)) {\n-\t\t\t\tstrncpy(ref_message_id, message_id,\n-\t\t\t\t\tsizeof(ref_message_id));\n-\t\t\t\tref_message_id[sizeof(ref_message_id)-1]='\\0';\n-\t\t\t\trev.ref_message_id = ref_message_id;\n+\t\t\tif (rev.message_id) {\n+\t\t\t\tif (rev.ref_message_id)\n+\t\t\t\t\tfree((char *) rev.message_id);\n+\t\t\t\telse\n+\t\t\t\t\trev.ref_message_id = rev.message_id;\n \t\t\t}\n-\t\t\tgen_message_id(message_id, sizeof(message_id),\n-\t\t\t\t       sha1_to_hex(commit->object.sha1));\n-\t\t\trev.message_id = message_id;\n+\t\t\tgen_message_id(&rev, sha1_to_hex(commit->object.sha1));\n \t\t}\n \t\tif (!use_stdout)\n \t\t\tif (reopen_stdout(commit, rev.nr, keep_subject,\n-- \n1.5.4.1.1350.g2b9ee\n"},{"id":"69133","messageId":"alpine.LSU.1.00.0802181238280.30505@racer.site","threadId":"12150","inReplyTo":"alpine.LNX.1.00.0802171335240.5816@iabervon.org","subject":"Re: [PATCH 1/4] Improve message-id generation flow control for format-patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-18T12:41:10Z","receivedAt":"2008-02-18T12:41:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 17 Feb 2008, Daniel Barkalow wrote:\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 99d69f0..867cc13 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -575,16 +575,19 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n>  \to2->flags = flags2;\n>  }\n>  \n> -static void gen_message_id(char *dest, unsigned int length, char *base)\n> +static void gen_message_id(struct rev_info *info, char *base)\n>  {\n>  \tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n>  \tconst char *email_start = strrchr(committer, '<');\n>  \tconst char *email_end = strrchr(committer, '>');\n> -\tif(!email_start || !email_end || email_start > email_end - 1)\n> +\tstruct strbuf buf;\n> +\tif (!email_start || !email_end || email_start > email_end - 1)\n>  \t\tdie(\"Could not extract email from committer identity.\");\n> -\tsnprintf(dest, length, \"%s.%lu.git.%.*s\", base,\n> -\t\t (unsigned long) time(NULL),\n> -\t\t (int)(email_end - email_start - 1), email_start + 1);\n> +\tstrbuf_init(&buf, 0);\n> +\tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n> +\t\t    (unsigned long) time(NULL),\n> +\t\t    (int)(email_end - email_start - 1), email_start + 1);\n> +\tinfo->message_id = strbuf_detach(&buf, NULL);\n\nWith this last line, and...\n\n> @@ -809,15 +810,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n>  \t\trev.nr = total - nr + (start_number - 1);\n>  \t\t/* Make the second and subsequent mails replies to the first */\n>  \t\tif (thread) {\n> -\t\t\tif (nr == (total - 2)) {\n> -\t\t\t\tstrncpy(ref_message_id, message_id,\n> -\t\t\t\t\tsizeof(ref_message_id));\n> -\t\t\t\tref_message_id[sizeof(ref_message_id)-1]='\\0';\n> -\t\t\t\trev.ref_message_id = ref_message_id;\n> +\t\t\tif (rev.message_id) {\n> +\t\t\t\tif (rev.ref_message_id)\n> +\t\t\t\t\tfree((char *) rev.message_id);\n\n... this one, you should make the message_id member of struct rev_info a \n\"char *\".  At least for this developer, \"const char *\" is a sign that the \ncaller should clean up, and that the pointer _might_ point to a constant.\n\nCiao,\nDscho\n"},{"id":"69177","messageId":"alpine.LNX.1.00.0802181229340.5816@iabervon.org","threadId":"12150","inReplyTo":"alpine.LSU.1.00.0802181238280.30505@racer.site","subject":"Re: [PATCH 1/4] Improve message-id generation flow control for format-patch","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-18T17:43:09Z","receivedAt":"2008-02-18T17:43:09Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 18 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Sun, 17 Feb 2008, Daniel Barkalow wrote:\n> \n> > diff --git a/builtin-log.c b/builtin-log.c\n> > index 99d69f0..867cc13 100644\n> > --- a/builtin-log.c\n> > +++ b/builtin-log.c\n> > @@ -575,16 +575,19 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha\n> >  \to2->flags = flags2;\n> >  }\n> >  \n> > -static void gen_message_id(char *dest, unsigned int length, char *base)\n> > +static void gen_message_id(struct rev_info *info, char *base)\n> >  {\n> >  \tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n> >  \tconst char *email_start = strrchr(committer, '<');\n> >  \tconst char *email_end = strrchr(committer, '>');\n> > -\tif(!email_start || !email_end || email_start > email_end - 1)\n> > +\tstruct strbuf buf;\n> > +\tif (!email_start || !email_end || email_start > email_end - 1)\n> >  \t\tdie(\"Could not extract email from committer identity.\");\n> > -\tsnprintf(dest, length, \"%s.%lu.git.%.*s\", base,\n> > -\t\t (unsigned long) time(NULL),\n> > -\t\t (int)(email_end - email_start - 1), email_start + 1);\n> > +\tstrbuf_init(&buf, 0);\n> > +\tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n> > +\t\t    (unsigned long) time(NULL),\n> > +\t\t    (int)(email_end - email_start - 1), email_start + 1);\n> > +\tinfo->message_id = strbuf_detach(&buf, NULL);\n> \n> With this last line, and...\n> \n> > @@ -809,15 +810,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> >  \t\trev.nr = total - nr + (start_number - 1);\n> >  \t\t/* Make the second and subsequent mails replies to the first */\n> >  \t\tif (thread) {\n> > -\t\t\tif (nr == (total - 2)) {\n> > -\t\t\t\tstrncpy(ref_message_id, message_id,\n> > -\t\t\t\t\tsizeof(ref_message_id));\n> > -\t\t\t\tref_message_id[sizeof(ref_message_id)-1]='\\0';\n> > -\t\t\t\trev.ref_message_id = ref_message_id;\n> > +\t\t\tif (rev.message_id) {\n> > +\t\t\t\tif (rev.ref_message_id)\n> > +\t\t\t\t\tfree((char *) rev.message_id);\n> \n> ... this one, you should make the message_id member of struct rev_info a \n> \"char *\".  At least for this developer, \"const char *\" is a sign that the \n> caller should clean up, and that the pointer _might_ point to a constant.\n\nIt's sort of like that, in that this *is* the caller, and it's using \ngen_message_id to set it and cleaning it up, and it could put in a \nconstant (in which case it would have to know this and not free it), but I \nagree that it's more suggestive of the right things as a \"char *\".\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"69181","messageId":"alpine.LSU.1.00.0802181759280.30505@racer.site","threadId":"12150","inReplyTo":"alpine.LNX.1.00.0802181229340.5816@iabervon.org","subject":"Re: [PATCH 1/4] Improve message-id generation flow control for format-patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-18T18:00:24Z","receivedAt":"2008-02-18T18:00:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 18 Feb 2008, Daniel Barkalow wrote:\n\n> It's sort of like that, in that this *is* the caller,\n\nSorry, I should have been clearer.  It is all too easy to forget that \nreasoning and use rev_info->message_id in a different way, and then mix \nthe code paths.\n\nCiao,\nDscho\n"}]}