Re: [PATCH 8/9] git_config_set: use do_config_from_file() directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 30, 2018 at 03:02:00PM +0200, Johannes Schindelin wrote:

> > I'm not sure I understand that last paragraph. What does flockfile() have
> > to do with stdin/stdout?
> > 
> > The point of those calls is that we're locking the FILE handle, so that
> > it's safe for the lower-level config code to run getc_unlocked(), which
> > is faster.
> > 
> > So without those, we're calling getc_unlocked() without holding the
> > lock. I think it probably works in practice because we know that we're
> > single-threaded, but it seems a bit sketchy.
> 
> Oops. I misunderstood the purpose of flockfile(), then. I thought it was
> only about multiple users of stdin/stdout.
> 
> Will have a look whether flockfile()/funlockfile() can be moved into
> do_config_from_file() instead.

In a sense stdin/stdout are much more susceptible to this because
they're global variables, and any thread may touch them. For the config
code, we open our own handle that we don't expose elsewhere. So probably
it would be fine just to use the unlocked variants even without locking.

But IMHO it's good practice to always flockfile() before using the
unlocked variants. My reading of POSIX is that it's OK to use the
unlocked variants without holding the lock (if you know there won't be
contention), but if it's not hard to err on the side of safety, I'd
prefer it.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux