xfsdump: don't assume getdents exists

common/getdents.c +10 -0

@@ -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;

Quoted 1 line

+++ b/common/getdents.c

Oh, how that code makes my eyes bleed. :/

Quoted 27 lines

common/getdents.c +10 -0

@@ -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)
     }
 

common/getdents.c getdents_wrap (int fd, char *buf, size_t nbytes) · line 279

Quoted 3 lines

common/getdents.c +1 -0

   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 <[email protected]>

Quoted 4 lines · Dave Chinner

Quoted 1 line

+++ b/common/getdents.c

Oh, how that code makes my eyes bleed. :/

Me too :\

Quoted 34 lines

Quoted 25 lines

common/getdents.c +10 -0

@@ -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;

common/getdents.c getdents_wrap (int fd, char *buf, size_t nbytes) · line 279

Quoted 5 lines

common/getdents.c +1 -0

@@ -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.

Quoted 1 line

Acked-by Dave Chinner <[email protected]>

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...)

Quoted 4 lines

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 ;)

New Linux ports are using a standard syscall list that does not include deprecated syscalls where 64-bit clean alternatives exist. As a result, on arm64, __NR_getdents is undefined, resulting in xfsdump failing to build.

To avoid that, in the case where __NR_getdents is unset in unistd.h, avoid building the fallback path entirely, since __ASSUME_GETDENTS64_SYSCALL will be true, the SYS_getdents64 case will be the primary (and only) path used.

Quoted 9 lines · Kyle McMartin

New Linux ports are using a standard syscall list that does not include deprecated syscalls where 64-bit clean alternatives exist. As a result, on arm64, __NR_getdents is undefined, resulting in xfsdump failing to build.

To avoid that, in the case where __NR_getdents is unset in unistd.h, avoid building the fallback path entirely, since __ASSUME_GETDENTS64_SYSCALL will be true, the SYS_getdents64 case will be the primary (and only) path used.

Seems like we could drop this mess of a file entirely and just rely on getdents64 from glibc. The raison d'etre of that file appears to support 64 bit getdents before glibc really supported it, and I don't think we'll have to support such old configurations anymore. I'll give it a quick spin and see how that works out.

New Linux ports are using a standard syscall list that does not include deprecated syscalls where 64-bit clean alternatives exist. As a result, on arm64, __NR_getdents is undefined, resulting in xfsdump failing to build.

To avoid that, in the case where __NR_getdents is unset in unistd.h, avoid building the fallback path entirely, since __ASSUME_GETDENTS64_SYSCALL will be true, the SYS_getdents64 case will be the primary (and only) path used.

Signed-off-by Kyle McMartin <[email protected]>

common/getdents.c +11 -0

@@ -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
 }

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Quoted 10 lines · Kyle McMartin

New Linux ports are using a standard syscall list that does not include deprecated syscalls where 64-bit clean alternatives exist. As a result, on arm64, __NR_getdents is undefined, resulting in xfsdump failing to build.

To avoid that, in the case where __NR_getdents is unset in unistd.h, avoid building the fallback path entirely, since __ASSUME_GETDENTS64_SYSCALL will be true, the SYS_getdents64 case will be the primary (and only) path used.

Quoted 1 line

Signed-off-by Kyle McMartin <[email protected]>

Hey Kyle - were you going to do a V2 of this one?

Just checking, thanks -
-Eric

Hey Kyle - were you going to do a V2 of this one?

Just checking, thanks -

Quoted 3 lines · Eric Sandeen

Hey Kyle - were you going to do a V2 of this one?

Just checking, thanks -

i've got a pair of somewhat more substantial patches to fix the ifdef hell in this file, clean things up so it works on both getdents64 only and not, and then rips out the pre-2.4.1 getdents code.

https://github.com/jkkm/xfsdump/commit/e156bcf06effbf5f2e26c53efb0ce028c4281f9e
https://github.com/jkkm/xfsdump/commit/fff56b471220a8fa305b9158c4f8f0c7344c8fe6

the latter looks quite a bit better... care to take a quick look and if they're decent, i'll submit them both.