[PATCH] s3/smbd: set FILE_ATTRIBUTE_DIRECTORY as necessary

[Christof Schmitt]

On Thu, Sep 15, 2016 at 07:21:45AM -0700, Christof Schmitt wrote:
> On Thu, Sep 15, 2016 at 11:47:54AM +0200, Ralph Böhme wrote:
> > 
> > On Wed, Sep 14, 2016 at 02:34:04PM -0700, Christof Schmitt wrote:
> > > On Wed, Sep 14, 2016 at 03:52:41PM +0200, Ralph Böhme wrote:
> > > > On Wed, Sep 14, 2016 at 03:42:04PM +0200, Ralph Böhme wrote:
> > > > > Hi!
> > > > > 
> > > > > Just stumbled across this one:
> > > > > 
> > > > > <https://bugzilla.samba.org/show_bug.cgi?id=12261>
> > > > > 
> > > > > Patch attached, please review & push if ok. Thanks!
> > > > 
> > > > sorry, here it is.
> > > > 
> > > > Cheerio!
> > > > -slow
> > > 
> > > > From 3246128f972a782543603a9d97516ddaa751d339 Mon Sep 17 00:00:00 2001
> > > > From: Ralph Boehme <slow at samba.org>
> > > > Date: Wed, 14 Sep 2016 12:52:48 +0200
> > > > Subject: [PATCH] s3/smbd: set FILE_ATTRIBUTE_DIRECTORY as necessary
> > > > 
> > > > Some VFS modules like GPFS will always return success from
> > > > SMB_VFS_GET_DOS_ATTRIBUTES() but only set a subset of the attributes. It
> > > > neither sets FILE_ATTRIBUTE_NORMAL nor FILE_ATTRIBUTE_DIRECTORY.
> > > 
> > > I just tested this and GPFS actually returns GPFS_WINATTR_DIRECTORY for
> > > directories. What is missing is the mapping to FILE_ATTRIBUTE_DIRECTORY
> > > in the vfs_gpfs module.
> > > 
> > > The patch still makes sense, but probably we should just add the missing
> > > mapping for completeness.
> > 
> > I was initially considering this as well, but then preferred the more
> > concise approach "one to rule them all", ie keep dealing with
> > FILE_ATTRIBUTE_NORMAL and FILE_ATTRIBUTE_DIRECTORY out of the modules,
> > but instead do it in one central point.
> > 
> > We're keeping FILE_ATTRIBUTE_NORMAL out the VFS modules, so imo it
> > makes more sense to keep FILE_ATTRIBUTE_DIRECTORY out of them as well.
> > 
> > What do you think?
> 
> Yes, makes sense. I just wanted to mention that this flag is actually
> returned, just not mapped. Keeping it out of the VFS modules makes
> sense, since the same information is already available in the stat
> information.

dos_mode_from_sbuf and get_ea_dos_attribute also set
FILE_ATTRIBUTE_DIRECTORY based on the stat info. While it does not hurt
to set it twice, should these two instances be removed when the same
logic is added in dosmode()?

Christof