From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Sun, 24 Sep 2017 21:52:54 +0100
Subject: [RFCv2 PATCH 5/7] ui-blame: pull blame info from libgit
In-Reply-To: <CAPX7N=6c2wMVq9r=MgfyS8uedunem9c4i5uTzrfUfDSSdTS=kQ@mail.gmail.com>
References: <20170608021810.12964-1-whydoubt@gmail.com>
 <20170923033848.5922-1-whydoubt@gmail.com>
 <20170923033848.5922-6-whydoubt@gmail.com>
 <20170923154758.GE2548@john.keeping.me.uk>
 <CAPX7N=4n2VFrX3DGZmkVdS2+4QLyE04t7XhXnDELKd7i5X3KxQ@mail.gmail.com>
 <CAPX7N=6c2wMVq9r=MgfyS8uedunem9c4i5uTzrfUfDSSdTS=kQ@mail.gmail.com>
Message-ID: <20170924205254.GH2548@john.keeping.me.uk>

On Sun, Sep 24, 2017 at 03:09:53PM -0500, Jeffrey Smith wrote:
> On Sun, Sep 24, 2017 at 2:06 PM, Jeffrey Smith <whydoubt at gmail.com> wrote:
> > A few of the original suggestions apparently got lost in the shuffle.
> > Anyway, it appears that making your suggested structure change
> > should clean some things up a lot.
> >
> > On Sat, Sep 23, 2017 at 10:47 AM, John Keeping <john at keeping.me.uk> wrote:
> >> On Fri, Sep 22, 2017 at 10:38:46PM -0500, Jeff Smith wrote:
> >>> Use the blame interface added in libgit to output the blame information
> >>> of a file in the repository.
> >>>
> >>> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
> >>> ---
> >>> diff --git a/ui-blame.c b/ui-blame.c
> >>> index 901ca89..cc4457a 100644
> >>> --- a/ui-blame.c
> >>> +++ b/ui-blame.c
> >>> +static void emit_blame_entry(struct blame_scoreboard *sb, struct blame_entry *ent)
> >>> +{
> >>> +     struct blame_origin *suspect = ent->suspect;
> >>> +     char hex[GIT_SHA1_HEXSZ + 1];
> >>> +     char *detail, *abbrev;
> >>> +     unsigned long lineno;
> >>> +     const char *numberfmt = "<a id='n%1$d' href='#n%1$d'>%1$d</a>\n";
> >>> +     const char *cp, *cpend;
> >>> +
> >>> +     oid_to_hex_r(hex, &suspect->commit->object.oid);
> >>> +     detail = emit_one_suspect_detail(suspect, hex);
> >>> +     abbrev = xstrdup(find_unique_abbrev(suspect->commit->object.oid.hash,
> >>> +                                         DEFAULT_ABBREV));
> >>
> >> nit: I don't think there's any need to strdup for abbrev, we use the
> >> result immediately so the static buffer won't get overwritten.
> 
> But find_unique_abbrev() returns a const char*, and abbrev is passed
> as the first parameter
> to cgit_commit_link(), which is char* (and the function can in fact
> change the contents).
> 
> I will add a patch to the series that avoids altering the first
> parameter of cgit_commit_link
> so that it will be const char*.

The strdup approach is probably better than requiring cgit_commit_link()
to make a copy of its argument.  I hadn't spotted that
find_unique_abbrev() returned a const char*.