Hi, On Tue, Dec 08, 2009 at 08:53:46PM +0200, Sergiu Ivanov wrote: > On Sun, Nov 22, 2009 at 09:05:16PM +0100, olafbuddenha...@gmx.net wrote: > > On Thu, Nov 19, 2009 at 10:28:37AM +0200, Sergiu Ivanov wrote:
> > > + /* Fetch the effective UIDs of the unionfs process. */ > > > + nuids = geteuids (0, 0); > > > + if (nuids < 0) > > > + return EPERM; > > > + uids = alloca (nuids * sizeof (uid_t)); > > > + > > > + nuids = geteuids (nuids, uids); > > > + assert (nuids > 0); > > > > Hrmph, I didn't spot this before: I don't think the assert() is right -- > > "nuids" (or "ngids") being exactly 0, is probably a perfectly valid > > case... And even if it is not, the test in the assert should be > > equivalent to the EPERM test above, to avoid confusion. > > OK, changed. For the record: We agreed on IRC that rather than changing the assert, it's better to go back to the original code, i.e. do the check/EPERM thing again. It is actually possible that the number of UIDs changes in the middle of things... (Yes Frederik, I agree that this is not ideal either :-) But fixing this properly is non-trivial, and out of scope here... Might be useful to file a bug on Savannah though so it won't get lost.) I'll just assume that the rest of the patch is also fine now, rather than looking through the whole thing again... I looked at it many times already; and even though it seems I'm spotting new things to nitpick about each time I look, I have neither time nor patience right now to do it yet another time. Also, I'm too afraid of actually discovering more stuff to nitpick about ;-) In other words: please push this to the main unionmount branch :-) -antrik-
