Re: [PATCH v3 0/4] MR951: ntdll: Add initial support for DOS file attributes (attempt 3)

[Erich Hoover (＠ehoover)]

On Tue Oct  4 16:56:07 2022 +0000, **** wrote:
> Zebediah Figura replied on the mailing list:
> On 9/30/22 11:42, Erich E. Hoover wrote:
> > +/* decode the xattr-stored DOS attributes */
> > +static int parse_samba_dos_attrib_data( char *data, int len )
> > +{
> > +    size_t string_len = strnlen( data, len );
> > +
> > +    if (len > string_len + 3)
> > +    {
> > +        uint16_t version;
> > +        memcpy( &version, data + string_len + 1, 2 );
> > +        version = ntohs(version);
> Are you sure this is correct? My reading of the code, having taken a 
> longer time to look at it, is that it's basically an NDR format string 
> containing, in order:
> * a null-terminated string (for compatibility with the old foramt)
> * align to 2 bytes
> * the xattr_DosInfo version
> * align to 4 bytes
> * the xattr_DosInfo version, again (because NDR)
> * the contents of the xattr_DosInfo structure
> I'm not a Samba expert and this is difficult to confirm without 
> familiarity with the code, but I don't think that the xattr are actually 
> stored locally as big-endian; probably that's just how they're 
> transmitted over the network. Confer my example from earlier:
> whatsit@camazotz:~/vmshare$ attr -q -g DOSATTRIB test-file | xxd -g4
> 00000000: 00000400 04000000 11000000 21000000  ............!...
> 00000010: 00000000 00000000 c27bfa02 65c8d801  .........{..e...
> The attributes themselves are clearly little-endian, and the version 
> numbers seem to be consistent with alignment rather than endianness 
> switching.
> > +        if ( version != 3 )
> > +        {
> > +            static BOOL warn = TRUE;
> > +            if (warn)
> > +            {
> > +                FIXME( "Unsupported Samba DOSATTRIB Extended
> Attribute version: %u\n",
> > +                       version );
> > +                warn = FALSE;
> > +            }
> > +        }
> We're not handing version 3 either, are we? Note that this seems to be 
> the xattr_DosInfo version, not the Samba version.

Rather than being smart about it, it might be better to just output an error when it's not a format we can decode.  Possibly something like:
```
    if (string_len > 2 && data[0] == '0' && data[1] == 'x')
        ...
    else
        FIXME_ONCE("Unhandled " SAMBA_XATTR_DOS_ATTRIB " extended attribute value.\n");
```