Jeff King <peff@xxxxxxxx> writes: > The string handling in link_alt_odb_entry() is mostly an > artifact of the original version, which took the path as a > ptr/len combo, and did not have a NUL-terminated string > until we created one in the alternate_object_database > struct. But since 5bdf0a8 (sha1_file: normalize alt_odb > path before comparing and storing, 2011-09-07), the first > thing we do is put the path into a strbuf, which gives us > some easy opportunities for cleanup. > > In particular: > > - we call strlen(pathbuf.buf), which is silly; we can look > at pathbuf.len. > > - even though we have a strbuf, we don't maintain its > "len" field when chomping extra slashes from the > end, and instead keep a separate "pfxlen" variable. We > can fix this and then drop "pfxlen" entirely. > > - we don't check whether the path is usable until after we > allocate the new struct, making extra cleanup work for > ourselves. Since we have a NUL-terminated string, we can > bump the "is it usable" checks higher in the function. > While we're at it, we can move that logic to its own > helper, which makes the flow of link_alt_odb_entry() > easier to follow. Also I find that this bit is a nice touch: > /* recursively add alternates */ > - read_info_alternates(ent->base, depth + 1); > - > - ent->base[pfxlen] = '/'; > + read_info_alternates(pathbuf.buf, depth + 1); We used to leave ent->base[] string terminated with NUL while calling read_info_alternates() and then added '/' after that, but because the new code uses a separate pathbuf for the call, ent->base[] can be prepared into the desired shape upfront. Much easier to follow.