Subject: Re: mkdir with trailing / (patch proposed)
To: enami tsugutomo <enami@but-b.or.jp>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-userlevel
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.

Take care,

Bill