[PATCH] Fix compilation error with vfs_glusterfs

[Anoop C S]

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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-vfs-Use-ENOATTR-in-errno-comparison-for-getxattr.patch
Type: text/x-patch
Size: 2574 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190123/d800fe09/0001-s3-vfs-Use-ENOATTR-in-errno-comparison-for-getxattr.bin>