On Mon, Jan 22, 2018 at 7:44 PM, Patryk Obara <patryk.obara@xxxxxxxxx> wrote: > On 22 January 2018 at 12:49, Duy Nguyen <pclouds@xxxxxxxxx> wrote: >> On Mon, Jan 22, 2018 at 12:04:28PM +0100, Patryk Obara wrote: >>> @@ -969,7 +969,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len, >>> >>> /* step 4: substitute */ >>> strbuf_addstr(buf, "Id: "); >>> - strbuf_add(buf, sha1_to_hex(sha1), 40); >>> + strbuf_add(buf, sha1_to_hex(oid.hash), GIT_SHA1_HEXSZ); >> >> oid_to_hex()? > > I didn't do it originally because the size of hash is explicitly > passed as the third parameter. Aha! I didn't see that. > I should probably replace this line with: > > strbuf_addstr(buf, oid_to_hex(&oid)); > > ... since a hex representation is correctly 0-delimited anyway. Yeah I think that's a good idea. This 40 came from 3fed15f568 (Add 'ident' conversion. - 2007-04-21). Back then memcpy was used and 40 made sense. The conversion to strbuf should have used strbuf_addstr() as you suggested, unless there's performance concerns, but I don't think there is any. -- Duy