On Fri, Jul 26, 2019 at 11:31 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > Returning before freeing the allocated buffer is suboptimal; as with > > elsewhere in the same function, make sure buf gets free'd. > > I do not have a real objection to the patch text, but the above > justification does not make much sense to me. The original code > returned an error when buf is NULL, so there is no leak returning > directly, without jumping to free_buf: label (whose only effect is > to free(buf) and return the value in ret). > > The real value of this change is it may future-proof the codepath by > making sure that everybody after an error goes to the same place to > free all resources---which happens to be only buf in the current > code, but this change allows it to change in the future, where some > additional resources may have been allocated before the call to > read_object_file() and freed after free_buf: label. Indeed, not sure how I overlooked that buf was NULL since that was the precise check I was modifying. I'll clean up the commit message.
