Jeff King <peff@xxxxxxxx> writes: > On Thu, Jun 10, 2021 at 09:06:44AM -0400, Jeff King wrote: > >> We can work around this by replacing our "did we hit the trailing NUL" >> subscript dereference with a length check. We do not even have to pay >> the cost for an extra strlen(), as we can pass our new length into >> interpret_branch_name(), which was converting our "0" into a call to >> strlen() anyway. >> [...] >> - if (0 < len && name[len] && buf.len) >> + if (0 < len && len < namelen && buf.len) >> strbuf_addstr(&buf, name + len); > > I guess another option would be to drop the check entirely. It is only > protecting us from calling strbuf_addstr() with an empty string, which > is a noop anyway (it would not even cause a useless allocation, since we > know that buf is non-empty, and that it won't need to grow). > > I think I still prefer my original solution, though. It may still work without the guard but it is not apparent to the readers if it just happens to work by accident or by design. At least the guard makes it clear what is going on, I would think.
