linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kmod: be32toh for older boxen
@ 2013-09-06  5:59 Randy MacLeod
  2013-09-06  5:59 ` [PATCH] kmod-native: Add back-up implementation of be32toh() Randy MacLeod
  0 siblings, 1 reply; 4+ messages in thread
From: Randy MacLeod @ 2013-09-06  5:59 UTC (permalink / raw)
  To: linux-modules; +Cc: randy.macleod

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] kmod-native: Add back-up implementation of be32toh().
  2013-09-06  5:59 [PATCH] kmod: be32toh for older boxen Randy MacLeod
@ 2013-09-06  5:59 ` Randy MacLeod
  2013-09-06 12:55   ` Lucas De Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Randy MacLeod @ 2013-09-06  5:59 UTC (permalink / raw)
  To: linux-modules; +Cc: 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.

Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] kmod-native: Add back-up implementation of be32toh().
  2013-09-06  5:59 ` [PATCH] kmod-native: Add back-up implementation of be32toh() Randy MacLeod
@ 2013-09-06 12:55   ` Lucas De Marchi
  2013-09-06 13:21     ` Randy MacLeod
  0 siblings, 1 reply; 4+ messages in thread
From: Lucas De Marchi @ 2013-09-06 12:55 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: linux-modules

Hi Randy,


On Fri, Sep 6, 2013 at 2:59 AM, Randy MacLeod
<Randy.MacLeod@windriver.com> wrote:
>
> 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.


>
> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>

We don't use s-o-b.


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


>
>  # 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

> +#if !defined(HAVE_BE32TOH) && !defined(be32toh)

as well as the double test.


> +#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?

> +#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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kmod-native: Add back-up implementation of be32toh().
  2013-09-06 12:55   ` Lucas De Marchi
@ 2013-09-06 13:21     ` Randy MacLeod
  0 siblings, 0 replies; 4+ messages in thread
From: Randy MacLeod @ 2013-09-06 13:21 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules

On 13-09-06 08:55 AM, Lucas De Marchi wrote:
> Hi Randy,
>
>
> On Fri, Sep 6, 2013 at 2:59 AM, Randy MacLeod
> <Randy.MacLeod@windriver.com> wrote:
>>
>> 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.
>
>
>>
>> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
>
> We don't use s-o-b.

ok.
>
>
>> ---
>>   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.
>
>
>>
>>   # 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
>
>> +#if !defined(HAVE_BE32TOH) && !defined(be32toh)
>
> as well as the double test.

Will do, simpler is better.

>
>
>> +#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.

>
>> +#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
>
> Thanks,
> Lucas De Marchi
>


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-09-06 13:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-06  5:59 [PATCH] kmod: be32toh for older boxen Randy MacLeod
2013-09-06  5:59 ` [PATCH] kmod-native: Add back-up implementation of be32toh() Randy MacLeod
2013-09-06 12:55   ` Lucas De Marchi
2013-09-06 13:21     ` Randy MacLeod

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).