This is the mail archive of the cygwin-developers@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ntea & chmod


At 11:24 AM 8/29/2004 +0200, Corinna Vinschen wrote:
>On Aug 28 23:38, Pierre A. Humblet wrote:
>> For example in ::fchmod, the line
>>       if (!set_file_attribute (pc.has_acls (), get_io_handle (), pc,
>>                                ILLEGAL_UID, ILLEGAL_GID, mode)
>>           && allow_ntsec)
>>         res = 0;
>> guarantees a -1 return when ntea is on and allow_ntsec is off.
>
>No, that's not right.  It gurantees a 0 return if set_file_attribute
>has returned success and ntsec is on.  The difference is that ntea
>depends on the return code of SetFileAttributes.  The `else' branch
>isn't quite correct, though.  It must be 
>
>  else if (!allow_ntsec || !pc.has_acls ())
>
>I fixed that.

OK

>> set_file_attribute (path_conv & pc ....   <=== new arg type
>> {
>>   int ret = 0;
>> 
>>   if (allow_ntsec && pc.has_acls ())
>>     ret = set_nt_attribute (handle, file, uid, gid, attribute);
>>   else if (allow_ntea && pc.fs_has_ea ())
>>     {
>>       SetFileAttributes (pc, (DWORD) pc & ~FILE_ATTRIBUTE_READONLY)
>>       bool res = NTWriteEA (file, ".UNIXATTR", (char *) &attribute,
>>                                      sizeof (attribute)))
>>       ....
>>      }
>>  
>> pc.fs_has_ea () should be set properly as well in path.cc.
>> Currently it's set to make sense for symlink (that's obsolete),
>
>Obsolete?  Hmm, you're right, PC_CHECK_EA is never set anywhere, AFAICS.
>Do we still need that code?

No, get/set_symlink_ea can be retired, or at least #if'ed out
and the tests about using them can be removed.

>> not for ntea.
>
>Yeah, it's only set for local NTFS.  It could also be set for FAT.
>But the above change wouldn't work in all circumstances, see mkdir()
>for instance;
>
>  if (!allow_ntsec && allow_ntea)
>    set_file_attribute (false, NULL, real_dir.get_win32 (), ...);
>                        ^^^^^
>except you keep the use_ntsec argument. 

With my approach this should be changed to 
(!(allow_ntsec && pc.has_acls()) && allow_ntea 
with the pc.fs_has_ea () test done in set_file_attribute
but as you write:

>I'm wondering though if it still makes sense to keep the
>get/set_file_attribute functions as they are.  Except for two cases,
>the functions are either called for the ntsec or for the ntea case
>exclusively.  Perhaps it would make things clearer again, if we use
>get/set_nt_attribute (resp. get_nt_object_attribute) and another
>function pair get/set_ea_attribute.

Yes, that's worth a try. We seem to duplicate the tests in the calling
function anyway.
In the ntea case, shouldn't the attribute be set in ::open when a file
is created, with the same kind of logic as in mkdir?

Pierre


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]