Subject: Re: mkdir with trailing / (patch proposed)
To: enami tsugutomo <firstname.lastname@example.org>
From: Bill Studenmund <email@example.com>
Date: 04/29/2002 15:48:36
On Tue, 30 Apr 2002, enami tsugutomo wrote:
> > On Tue, 30 Apr 2002, enami tsugutomo wrote:
> > > There is my version in PR15397.
> > Your patch in 15397 seems to include a bit more than just fixing this bug;
> The patch is to address 15397, not just mkdir(2) :-)
> > you seem to be doing some explicit trailing '0' which should be factored
> > out. We should probably just do the trailing '0' changes. :-)
> But this is also necessary for mkdir(2) case. Currently, fs code
> assumes that the last components is NUL terminated (but not always,
> since lookup code works with trailing /). I think just always trust
> the length of component instead is consistient. Of course, this
> changes protocol between each fs and fs independent layer (but is it
> well defined?)
Is it? I agree that the memcpy code is exactly the memcpy code you'd use
for a NUL terminated string. But for example ufs only looks at cn_namelen
characters. It copies cn_namelen + 1, but only ever looks at cn_namelen of
them. While adding a trailing '\0' might make dumps look nice, I don't
think we need it.
> > We picked a different name (NOREQDIR vs CREATEDIR), and a different bit.
> > I think the lower bit is slightly better as it's grouped with the other
> > things (LOCKPARENT), etc. that get set on the way in, not the bits that we
> > set internally (the lower bit is in MODMASK, which is commented as /* mask
> > of operational modifiers */).
> Ya, I just didn't notice the MODMASK. BTW, as commented, my choise
> CREATEDIR is intended that ``we are creating new directory entry and
> it is a directory''.
Is there ever anything else we would need this for? The reason I didn't
use "CREATE" in the name was the error test doesn't have CREATE in it
either. It just seems a bit weird for RENAME to be adding CREATEDIR. :-)
> > The one major difference is that your patch doesn't check to see if we're
> > at the last component name. Consider mkdir("/tmp/foo/bar/") when /tmp
> > exists and /tmp/foo does not; we still want to have the system call error
> > out.
> Note that at the point error is EJUSTRETURN. If a component in
> question was not last one, VOP_LOOKUP will return real error instead
> of it, won't it?
Doh. True. So that test isn't needed.