Michael Haggerty wrote: > --- a/lockfile.c > +++ b/lockfile.c > @@ -311,12 +311,14 @@ int reopen_lock_file(struct lock_file *lk) > int commit_lock_file(struct lock_file *lk) > { > char result_file[PATH_MAX]; > - size_t i; > + > if (lk->fd >= 0 && close_lock_file(lk)) > return -1; > + > strcpy(result_file, lk->filename); > - i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */ > - result_file[i] = 0; > + /* remove ".lock": */ > + result_file[strlen(result_file) - LOCK_SUFFIX_LEN] = 0; > + Same comment as the delete_ref_loose patch: the reader can gain some piece of mind from an assertion at the top that makes sure filename is valid (which it always should be, according to the API): assert(lk->filename[0]); It would also be nice to use the same xmemdupz-based code as in the delete_ref_loose case (ideally via a helper), to avoid having to work with a PATH_MAX-sized buffer. Otherwise looks good. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html