http://git.samba.org/?p=tprouty/samba.git; a=shortlog;
h=dos_attr_work
Tim Prouty
tprouty at samba.org
Tue Nov 11 18:31:33 GMT 2008
Hi James,
Thanks for the feedback!
On Nov 10, 2008, at 5:18 PM, James Peach wrote:
> 1. You should #ifdef some of the UF_* constants, because lots of
> systems don't have more than the minimal set or use different names.
> The names you have don't seem to be the standard BSD names.
We have extended the standard bsd inode and stat st_flags field to add
support for getting/setting CIFS attributes, which is why the names
are not standard BSD names. They are #ifdef'd by the configure check
(which is justified below), so that systems using a different system
for getting/setting attributes are not affected.
> 2. When you are getting the flags, you should also check the SF_*
> equivalents, eg, SF_IMMUTABLE.
I'm only interested in the special windows specific attributes when
getting getting the flags (UF_DOS_*) of which there are no SF_*
equivalents. If the SF_IMMUTABLE flag is set, the VOP_SETATTR
implementation of the file system will decide what to do when setting
the flags with chflags. The standard behavior is to return EPERM,
which is translated to NT_STATUS_ACCESS_DENIED. This seems
reasonable, so I'm not sure what a user-space check for SF_IMMUTABLE
would provide.
> 3. The DOS archive bit is actually the inverse of the BSD archive bit.
> Although the standard BSD mit is UF_NODUMP, so maybe UF_DOS_ARCHIVE is
> different?
Yes, this implementation of storing DOS attributes adds new flags in
order to avoid overloading the meaning of existing unix flags.
> 4. My patch also checks UF_IMMUTABLE in can_access_file() ... not sure
> whether you would still need that.
For the same reason of not checking for SF_IMMUTABLE, I don't think a
check for UF_IMMUTABLE is necessary in this case.
> 5. You don't need the configure check at all. SMB_VFS_CHFLAGS takes
> care of portably setting the flags and there is already a check for
> the st_flags field. Just wrap the usages of UD_DOS_* in #ifdefs.
I'm trying to write this in such a way that other people could add
support for storing dos attributes in the st_flags for other operating
systems. With this in mind it seems cleaner to make this a configure
check, and use the result for the #ifdef checks. This way the ability
to store dos attributes in st_flags is either on or off.
> 6. The change to proto.h seems spurious?
I added it for another patch I'm working on, that uses the helper
function. I agree it is spurious at this point, and I'll make it part
of the other patch that utilizes it, rather than including it as part
of this patch.
-Tim
More information about the samba-technical
mailing list