[PATCH] Fix compilation error with vfs_glusterfs

[Uri Simchoni]

On 1/23/19 12:59 PM, Anoop C S via samba-technical wrote:
> On Wed, 2019-01-23 at 11:30 +0200, Uri Simchoni via samba-technical
> wrote:
>> 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.
> 
> I agree.
> 
>> 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.
> 
> What about the attached version?
> 

RB+ me.

Thanks,
Uri