Re: [PATCH 2/2] close_lock_file(): new function in the lockfile API
- From
Brandon Casey <casey@nrlssc.navy.mil>
- Date
- Jan 16, 2008, 23:08 UTC
- Message-ID
- <478E8E6B.80702@nrlssc.navy.mil>
- In-Reply-To
- <7vy7apmlci.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
Show 6 quoted lines
> Brandon Casey <casey@nrlssc.navy.mil> writes: > >> Mainly, I prefer to not modify the data structures when a failure occurs. > > Ok. Is the rest of your patch that fixes callers Ok with that > semantics?
yes.
> If so, I'd agree that is probably cleaner. I'll > scrap the one we are discussing, resurrecting only the api > documentation part, and replace it with the lockfile.c changes > from your patch, along with the fixes to callers.
Most of that patch is straight forward, just removing close().
I think you should consider how to handle fdopen on the lock descriptor and the fact that start_command closes the lock file descriptor in create_bundle().
After we fdopen, we should always fclose() and never close(). This isn't enforced.
I merely assigned the file descriptor to -1 when it was safe (i.e. after fclose), and added a comment. We could add another function which did this automatically, but maybe that is too much effort, especially in the bundle case.
-brandon