Michael Haggerty wrote: > We could probably continue to use the filename field to encode the > state by being careful to write characters 1..N-1 of the filename > first, and then overwrite the NUL at filename[0] with the first > character of the filename, but that would be awkward and error-prone. > > So, instead of using the filename field to determine whether the > lock_file object is active, add a new field "lock_file::active" for > this purpose. Nice. [...] > --- a/cache.h > +++ b/cache.h > @@ -576,6 +576,7 @@ extern int refresh_index(struct index_state *, unsigned int flags, const struct > > struct lock_file { > struct lock_file *next; > + volatile sig_atomic_t active; > int fd; > pid_t owner; > char on_list; [...] > +++ b/lockfile.c > @@ -27,16 +27,19 @@ [...] > @@ -189,9 +198,14 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) > atexit(remove_lock_file); > } > > + if (lk->active) > + die("BUG: lock_file(\"%s\") called with an active lock_file object", > + path); The error message doesn't make it entirely obvious to me what went wrong. Maybe something like die("BUG: cannot lock_file(\"%s\") on active struct lock_file", path); lock_file already assumed on_list was initialized to zero but it wasn't particularly obvious since everything else is blindly scribbled over. Probably worth mentioning in the API docs that the lock_file should be zeroed before calling hold_lock_file_... [...] > @@ -326,6 +341,7 @@ int commit_lock_file(struct lock_file *lk) > if (rename(lk->filename, result_file)) > goto rollback; > > + lk->active = 0; > lk->filename[0] = 0; Is it useful to set filename[0] any more? It seems potentially fragile to set both, since new code can appear that only sets or checks one or the other. Would it make sense to rename the filename field to make sure no new callers relying on the filename[0] convention sneak in (with the new convention being that the filename doesn't get cleared, to avoid problems)? 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