Issue with changes to VFS_STAT and VFS_LSTAT.

[Tim Prouty]

Jeremy, thank you for bringing this up!  It was an issue I was  
planning on addressing at some point during this conversion, but  
there's no reason not to just address it now.  My responses are inline  
below.

On Jun 26, 2009, at 3:47 PM, Jeremy Allison wrote:

> This pushes the 'struct smb_filename' into visibility at the
> POSIX VFS layer, which as a shorthand to keep the stat struct
> tied to the filename is fine.
>
> However, in the default stat() implementation it now calls :
>
> get_full_smb_filename() which concatinates the base name + stream
> name (if one exists). Which means potentially the full stream
> names are visible to an underlying POSIX system than cannot
> cope with them.

This is actually true for vfswrap_open() as well, which has a comment  
that is basically asking the same question.

> Is this desired ? The logic in the upper level smbd should
> ensure that basic POSIX stat is containing a stream name is never
> called if the underlying filesystem can't support them.

My reason for adding the call to get_full_smb_filename() in  
vfs_default.c was to maintain the existing behavior of passing the  
full name to the posix call if no modules implementing streams first  
handled the call.  This would be the case if smbd were run without  
specifying any streams modules.

Previously there was no correct way at the vfs layer to differentiate  
between a filename containing a colon (posix extensions, demangled  
name) and a stream as passed over the wire by the windows client.  Now  
that there is more detailed information about the filename at the  
posix level, I fully agree that this is a good time to diverge from  
previous behavior.

> My feeling is in the "default" module case which remember is
> the last stop before hitting a *pure* "POSIX" filesystem we should
> do an SMB_ASSERT() that smb_fname->stream_name == NULL, as I can't
> see any case where the "default" POSIX filesystem can do
> anything but error out here. It actually is exposing a logic error
> in the upper level logic IMHO. A simple patch for this follows.

In the case of smbd running with no streams modules, I believe your  
patch will cause an assert to be fired for every smb_filename-based  
vfs op.  What do you think about returning an error and setting ERRNO  
to something like ENOENT if stream != NULL?  We could even go one step  
further and allow the vfs_default posix wrappers to allow opening  
the ::$DEFAULT stream by checking is_ntfs_default_stream_smb_fname()  
and using the base_name if it returns true.

I'm happy to make these changes in my next round of the migration.

Also, thank you for following all of these changes!  I'm touching a  
lot of code, and it's good to know you're keeping an eye on them :).

-Tim