[PATCH] Fix compilation error with vfs_glusterfs

[Uri Simchoni]

On 1/23/19 8:36 AM, Martin Schwenke via samba-technical wrote:
> On Wed, 23 Jan 2019 08:22:49 +0200, Uri Simchoni via samba-technical
> <samba-technical at lists.samba.org> wrote:
> 
>> On 1/23/19 5:54 AM, Anoop C S via samba-technical wrote:
>>> On Wed, 2019-01-23 at 11:58 +1100, Amitay Isaacs wrote:  
>>>> Hi Anoop,
>>>>
>>>> On Wed, Jan 23, 2019 at 4:03 AM Anoop C S via samba-technical
>>>> <samba-technical at lists.samba.org> wrote:  
>>  [...]  
>>  [...]  
>>  [...]  
>>  [...]  
>>  [...]  
>>  [...]  
>>  [...]  
>>>>
>>>> While you are fixing this issue, would you also fix the build problem
>>>> on freebsd?
>>>>
>>>> [3398/3906] Compiling source3/modules/vfs_glusterfs_fuse.c
>>>> ../../source3/modules/vfs_glusterfs_fuse.c:45:16: error: use of
>>>> undeclared identifier 'ENODATA'
>>>>                 if (errno == ENODATA) {
>>>>                              ^
>>>> 1 error generated.
>>>>
>>>> As you can see, on freebsd ENODATA is not defined.  
>>>
>>> Please see the attached patch which adds a generic #ifndef for ENODATA.
>>> I hope this need not be platform specific especially in this particular
>>> context where error returned for getxattr() is validated.
>>>
>>> I have also added check for ENOATTR considering platforms which
>>> directly return ENOATTR for getxattr() in the absence of an extended
>>> attribute. ENOATTR should be always defined based on the following
>>> #ifndef from source3/includes/includes.h
>>>
>>> ...
>>> #ifndef ENOATTR
>>> #if defined(ENODATA)
>>> #define ENOATTR ENODATA
>>> #else
>>> #define ENOATTR ENOENT
>>> #endif
>>> #endif
>>> ...
>>>   
>>
>> Why are we comparing to ENODATA in the first place? Why not just compare
>> to ENOATTR? The man pages I've been able to get my hands on all list
>> ENOATTR as the "no such attribute" errno. On Linux, ENOATTR is defined
>> to be ENODATA.
> 
> I wondered about that.  So then I wondered about coming up with a patch
> to remove all uses of ENODATA in the tree... but then when I noticed
> that Linux defines ENODATA, and then defines ENOATTR to be ENODATA in
> <attr/xattr.h>, my brain imploded and I decided I didn't want to start
> digging that hole right now.  ;-)
> 
> peace & happiness,
> martin
> 

It may not be such a deep hole because it's about glusterfs, not generic
server code that runs on any OS out there. Considering only Linux and
FreeBSD, ENOATTR seems like the way to go here.

It really looks like a programming mistake in the original code that is
getting propagated instead of corrected.

Another point is that if you want to fix ENODATA, you need to fix it in
source3/modules/posixacl_xattr.c as well - that's code common to
glusterfs and ceph.

Thanks,
Uri