Re: [PATCH] xfsdump: don't assume getdents exists

[Dave Chinner <david@fromorbit.com>]

On Wed, Nov 06, 2013 at 05:56:20PM -0500, Kyle McMartin wrote:
> On Thu, Nov 07, 2013 at 09:48:44AM +1100, Dave Chinner wrote:
> > > +++ b/common/getdents.c
> > 
> > Oh, how that code makes my eyes bleed. :/
> > 
> 
> Me too :\
> 
> > > @@ -70,6 +70,9 @@ extern int __have_no_getdents64;
> > >  # ifndef SYS_getdents64
> > >  #  define SYS_getdents64 __NR_getdents64
> > >  # endif
> > > +# ifndef __NR_getdents
> > > +#  define __ONLY_GETDENTS64_SYSCALL     1
> > > +# endif
> > >  #endif
> > >  
> > >  
> > > @@ -207,6 +210,13 @@ getdents_wrap (int fd, char *buf, size_t nbytes)
> > >  # endif
> > >      }
> > >  #endif
> > > +/* Newer Linux ports are not adding deprecated syscalls, so to avoid compile
> > > + * failures since SYS_getdents will be undefined, we check for that and only
> > > + * build the fall-back case if SYS_getdents is defined. We know that
> > > + * __ASSUME_GETDENTS64_SYSCALL is set, since __NR_getdents64 must exist on
> > > + * those platforms.
> > > + */
> > > +#ifndef __ONLY_GETDENTS64_SYSCALL
> > >    {
> > >      size_t red_nbytes;
> > >      struct kernel_dirent *skdp, *kdp;
> > > @@ -265,4 +275,5 @@ getdents_wrap (int fd, char *buf, size_t nbytes)
> > >      }
> > >  
> > >    return (char *) dp - buf;
> > > +#endif
> > >  }
> > 
> > It's not pretty - but that code has already broken the ugly stick so
> > it isn't making the situation any worse.
> > 
> > Acked-by: Dave Chinner <dchinner@redhat.com>
> 
> I'll try to clean it up and send a patch for that, using getdents64
> wherever possible, and handling the fallback at runtime instead of
> making things all ifdeffy... Just wanted to get things building in the
> mean time. (Oh man the coding style there is janky...)

I'd factor and reformat the code using the normal kernel style. :)

And to avoid ifdef hackery and runtime fallbacks, I'd add an
autoconf macro to determine which getdents call is supported, and
then only compile in the relevant function and not have to care
about runtime fallbacks.

But that may end up more complex and more work than you had in mind,
so feel free to ignore my suggestions ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs