This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Races in group/passwd code (was Re: etc_changed, passwd & group)


Christopher Faylor wrote:
> 
> 
> Btw, Pierre, can you explain the rationale behind the "check" parameter
> that some of the internal_* functions take?  Why would you not want to
> check for an up-to-date /etc/passwd or /etc/group?  

Two reasons:
1) "check" is only true when the internal functions are called from external
functions. getpwuid and non-reentrant friends return pointers to the malloced
copy. They have to remain valid at least until the next non-reentrant call,
thus cannot be invalidated by refresh from internal lookup functions.
This could be solved with per thread storage, but it's more expensive,
maximum size becomes an issue, etc...
2) internal lookups are used a lot to map uids from/to sids, so decreasing
overhead for them looks like a good idea to make Cygwin faster.   

> If not for this, it
> seems like we could get rid of the initialization check entirely and
> just let fn_changed return true when fn[etc_ix] is uninitialized,
> forcing an initial read/parse of the appropriate file.

That's true for the "uninitialized" state (although it's a cheaper comparison)
but not for the "initializing" state, at least with the current locking
scheme. That condition is checked twice (before and after the mutex), whereas
WaitForSingleEvent and fn_changed normally return false the second time 
around even when true the first time. Thus it's necessary to remember that they 
were true the first time.
Given that we have to remember something, it's cheap to use the same variable
to find out that we are uninitialized.
Recall too that in my book fn[etc_itx] is duplicative thus unnecessary :) 

Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]