[PATCH] kmod: be32toh for older boxen

Discussion:

Randy MacLeod

2013-09-06 05:59:00 UTC

The attached patch fixes kmod when built on RHEL-5.9.
I need to do this for a yocto cross-compile.
I've also built on Ubuntu-12.04.

No run-time testing as the change seems simple enough.

Let me now if any changes are needed since this is my first
patch to kmod.

Thanks,

// Randy

Randy MacLeod (1):
kmod-native: Add back-up implementation of be32toh().

configure.ac | 1 +
libkmod/libkmod-signature.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)

--
1.8.2.1

Randy MacLeod

2013-09-06 05:59:01 UTC

Permalink

Older hosts may not have the be32toh function defined. Check for this and
fall back to checking the endianness and calling bswap_32 directly if needed.
This works on both old and new hosts.

Signed-off-by: Randy MacLeod <Randy.MacLeod-***@public.gmane.org>
---
configure.ac | 1 +
libkmod/libkmod-signature.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 40e54cf..de2ee49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -45,6 +45,7 @@ PKG_PROG_PKG_CONFIG
AC_CHECK_FUNCS_ONCE(__xstat)
AC_CHECK_FUNCS_ONCE([__secure_getenv secure_getenv])
AC_CHECK_FUNCS_ONCE([finit_module])
+AC_CHECK_FUNCS_ONCE([be32toh])

# dietlibc doesn't have st.st_mtim struct member
AC_CHECK_MEMBERS([struct stat.st_mtim], [], [], [#include <sys/stat.h>])
diff --git a/libkmod/libkmod-signature.c b/libkmod/libkmod-signature.c
index 6237ab7..791d092 100644
--- a/libkmod/libkmod-signature.c
+++ b/libkmod/libkmod-signature.c
@@ -18,7 +18,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

-#include <endian.h>
+#include <byteswap.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
@@ -26,6 +26,18 @@

#include "libkmod-internal.h"

+/* be32toh is usually a macro definend in <endian.h>, but it might be
+ * a function in some system so check both, and if neither is defined
+ * then define be32toh (for RHEL 5).
+ */
+#if !defined(HAVE_BE32TOH) && !defined(be32toh)
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define be32toh(x) __bswap_32 (x)
+#else
+#define be32toh(x) (x)
+#endif
+#endif
+
/* These types and tables were copied from the 3.7 kernel sources.
* As this is just description of the signature format, it should not be
* considered derived work (so libkmod can use the LGPL license).

--
1.8.2.1

Lucas De Marchi

2013-09-06 12:55:08 UTC

Permalink

Hi Randy,

On Fri, Sep 6, 2013 at 2:59 AM, Randy MacLeod

Post by Randy MacLeod
Older hosts may not have the be32toh function defined. Check for this and
fall back to checking the endianness and calling bswap_32 directly if needed.
This works on both old and new hosts.

I'm fine with adding fallback code, even if I'm surprised with it.
However see comments below.
We don't use s-o-b.

Post by Randy MacLeod
---
configure.ac | 1 +
libkmod/libkmod-signature.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index 40e54cf..de2ee49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -45,6 +45,7 @@ PKG_PROG_PKG_CONFIG
AC_CHECK_FUNCS_ONCE(__xstat)
AC_CHECK_FUNCS_ONCE([__secure_getenv secure_getenv])
AC_CHECK_FUNCS_ONCE([finit_module])
+AC_CHECK_FUNCS_ONCE([be32toh])

I guess you could use AC_CHECK_DECLS_ONCE so the check below is
simpler, since you don't need to care if it's a macro or a function.

Post by Randy MacLeod
# dietlibc doesn't have st.st_mtim struct member
AC_CHECK_MEMBERS([struct stat.st_mtim], [], [], [#include <sys/stat.h>])
diff --git a/libkmod/libkmod-signature.c b/libkmod/libkmod-signature.c
index 6237ab7..791d092 100644
--- a/libkmod/libkmod-signature.c
+++ b/libkmod/libkmod-signature.c
@@ -18,7 +18,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include <endian.h>
+#include <byteswap.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
@@ -26,6 +26,18 @@
#include "libkmod-internal.h"
+/* be32toh is usually a macro definend in <endian.h>, but it might be
+ * a function in some system so check both, and if neither is defined
+ * then define be32toh (for RHEL 5).
+ */

by using AC_CHECK_DECLS_ONCE this comment would be gone

Post by Randy MacLeod
+#if !defined(HAVE_BE32TOH) && !defined(be32toh)

as well as the double test.

Post by Randy MacLeod
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define be32toh(x) __bswap_32 (x)

Is there any reason to include byteswap.h and then end up using the
__bswap* variant?

Post by Randy MacLeod
+#else
+#define be32toh(x) (x)
+#endif
+#endif
+

Please add this check to the missing.h header. We may want to use this
function in more places. Then in this file you just need to include
this file.

Thanks,
Lucas De Marchi

Randy MacLeod

2013-09-06 13:21:30 UTC

Permalink

Post by Lucas De Marchi
Hi Randy,
On Fri, Sep 6, 2013 at 2:59 AM, Randy MacLeod

I'm fine with adding fallback code, even if I'm surprised with it.
However see comments below.
We don't use s-o-b.

ok.

Post by Lucas De Marchi

I guess you could use AC_CHECK_DECLS_ONCE so the check below is
simpler, since you don't need to care if it's a macro or a function.

by using AC_CHECK_DECLS_ONCE this comment would be gone

Post by Randy MacLeod
+#if !defined(HAVE_BE32TOH) && !defined(be32toh)

as well as the double test.

Will do, simpler is better.

Post by Lucas De Marchi

Post by Randy MacLeod
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define be32toh(x) __bswap_32 (x)

Is there any reason to include byteswap.h and then end up using the
__bswap* variant?

No, it was late and I wanted to know if you guys would
take a patch to make kmod build on old hosts so I
sent this a little early.

Post by Lucas De Marchi

Post by Randy MacLeod
+#else
+#define be32toh(x) (x)
+#endif
+#endif
+

Please add this check to the missing.h header. We may want to use this
function in more places. Then in this file you just need to include
this file.

Ok.
Thanks for the review.
v2 coming later today.

// Randy

Post by Lucas De Marchi
Thanks,
Lucas De Marchi

--
# Randy MacLeod. SMTS, Linux, Wind River
Direct: 613.963.1350