Discussion:
[Ipmitool-devel] [PATCH] Add bswap.h to ipmi_chassis.c and ipmi_pef.c
Dan Gora
2013-03-22 00:08:16 UTC
Permalink
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.

Signed-off-by: Dan Gora <***@adax.com>
---
ipmitool/lib/ipmi_chassis.c | 1 +
ipmitool/lib/ipmi_pef.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/ipmitool/lib/ipmi_chassis.c b/ipmitool/lib/ipmi_chassis.c
index 9f71d32..f0d4877 100644
--- a/ipmitool/lib/ipmi_chassis.c
+++ b/ipmitool/lib/ipmi_chassis.c
@@ -45,6 +45,7 @@
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_strings.h>
#include <ipmitool/ipmi_chassis.h>
+#include <ipmitool/bswap.h>

extern int verbose;

diff --git a/ipmitool/lib/ipmi_pef.c b/ipmitool/lib/ipmi_pef.c
index 1c0e378..2228b91 100644
--- a/ipmitool/lib/ipmi_pef.c
+++ b/ipmitool/lib/ipmi_pef.c
@@ -43,6 +43,7 @@
#include <ipmitool/ipmi.h>
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_pef.h>
+#include <ipmitool/bswap.h>

extern int verbose;
/*
--
1.7.7
Zdenek Styblik
2013-04-04 11:57:38 UTC
Permalink
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.

Z.
Post by Dan Gora
---
ipmitool/lib/ipmi_chassis.c | 1 +
ipmitool/lib/ipmi_pef.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/ipmitool/lib/ipmi_chassis.c b/ipmitool/lib/ipmi_chassis.c
index 9f71d32..f0d4877 100644
--- a/ipmitool/lib/ipmi_chassis.c
+++ b/ipmitool/lib/ipmi_chassis.c
@@ -45,6 +45,7 @@
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_strings.h>
#include <ipmitool/ipmi_chassis.h>
+#include <ipmitool/bswap.h>
extern int verbose;
diff --git a/ipmitool/lib/ipmi_pef.c b/ipmitool/lib/ipmi_pef.c
index 1c0e378..2228b91 100644
--- a/ipmitool/lib/ipmi_pef.c
+++ b/ipmitool/lib/ipmi_pef.c
@@ -43,6 +43,7 @@
#include <ipmitool/ipmi.h>
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_pef.h>
+#include <ipmitool/bswap.h>
extern int verbose;
/*
--
1.7.7
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
Dan Gora
2013-04-04 17:19:35 UTC
Permalink
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?

thanks
dan
Dan Gora
2013-04-04 17:21:16 UTC
Permalink
Ok, never mind.. it was.. Sorry about that.. I originally did all the
changes against 1.8.12, then ported them to the tip of the CVS tree.
I didn't see that this was already fixed.

thanks
dan
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?
thanks
dan
Zdenek Styblik
2013-04-04 19:14:14 UTC
Permalink
Post by Dan Gora
Ok, never mind.. it was.. Sorry about that.. I originally did all the
changes against 1.8.12, then ported them to the tip of the CVS tree.
I didn't see that this was already fixed.
thanks
dan
No worries.

Z.
Post by Dan Gora
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?
thanks
dan
Dan Gora
2013-04-24 21:04:46 UTC
Permalink
Hi Zdenek,

It turns out that this bug is not a duplicate after all.. The TOB of
the CVS tree as of today still has the same compilation failure on big
endian machines.

The problem is the include files in ipmi_pef.c and ipmi_chassis.c

#include <string.h>
#include <math.h>
#include <time.h>

#if WORDS_BIGENDIAN
# include <ipmitool/bswap.h>
#endif

#include <ipmitool/helper.h>
#include <ipmitool/log.h>
#include <ipmitool/ipmi.h>
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_pef.h>


In this case, bswap.h doesn't get included because there are no other
header files before it which include <config.h>, so WORDS_BIGENDIAN is
not defined until ipmi.h is included.

The easiest solution is just to remove the #if WORDS_BIGENDIAN from
around the #include <ipmitool/bswap.h>.

thanks
dan
Post by Zdenek Styblik
Post by Dan Gora
Ok, never mind.. it was.. Sorry about that.. I originally did all the
changes against 1.8.12, then ported them to the tip of the CVS tree.
I didn't see that this was already fixed.
thanks
dan
No worries.
Z.
Post by Dan Gora
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?
thanks
dan
Dan Gora
2013-04-24 22:16:52 UTC
Permalink
Here's a quick patch to fix this.

thanks
dan

commit b1f38c34edc679eac03613b6ceb5825deca2e854
Author: Dan Gora <***@adax.com>
Date: Wed Apr 24 19:09:54 2013 -0300

[debian] ipmitool: Fix compilation bug on big endian machines.

On big endian machines the compilation would fail because BSWAP_32
was not defined in ipmi_chassis.c and ipmi_pef.c.

The problem was including bswap.h depended on WORDS_BIGENDIAN which
comes from <config.h>, however none of the previous header files
would include config.h, so it remained undefined.

We now just include the bswap.h unconditionally.

diff --git a/linux/debian/ipmitool/lib/ipmi_chassis.c
b/linux/debian/ipmitool/lib/ipmi_chassis.c
index 9f71d32..aaf3878 100644
--- a/linux/debian/ipmitool/lib/ipmi_chassis.c
+++ b/linux/debian/ipmitool/lib/ipmi_chassis.c
@@ -35,10 +35,7 @@
#include <stdio.h>
#include <time.h>

-#if WORDS_BIGENDIAN
-# include <ipmitool/bswap.h>
-#endif
-
+#include <ipmitool/bswap.h>
#include <ipmitool/helper.h>
#include <ipmitool/ipmi.h>
#include <ipmitool/log.h>
diff --git a/linux/debian/ipmitool/lib/ipmi_pef.c
b/linux/debian/ipmitool/lib/ipmi_pef.c
index c0fa00d..154bf40 100644
--- a/linux/debian/ipmitool/lib/ipmi_pef.c
+++ b/linux/debian/ipmitool/lib/ipmi_pef.c
@@ -34,10 +34,7 @@
#include <math.h>
#include <time.h>

-#if WORDS_BIGENDIAN
-# include <ipmitool/bswap.h>
-#endif
-
+#include <ipmitool/bswap.h>
#include <ipmitool/helper.h>
#include <ipmitool/log.h>
#include <ipmitool/ipmi.h>
Post by Dan Gora
Hi Zdenek,
It turns out that this bug is not a duplicate after all.. The TOB of
the CVS tree as of today still has the same compilation failure on big
endian machines.
The problem is the include files in ipmi_pef.c and ipmi_chassis.c
#include <string.h>
#include <math.h>
#include <time.h>
#if WORDS_BIGENDIAN
# include <ipmitool/bswap.h>
#endif
#include <ipmitool/helper.h>
#include <ipmitool/log.h>
#include <ipmitool/ipmi.h>
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_pef.h>
In this case, bswap.h doesn't get included because there are no other
header files before it which include <config.h>, so WORDS_BIGENDIAN is
not defined until ipmi.h is included.
The easiest solution is just to remove the #if WORDS_BIGENDIAN from
around the #include <ipmitool/bswap.h>.
thanks
dan
Post by Zdenek Styblik
Post by Dan Gora
Ok, never mind.. it was.. Sorry about that.. I originally did all the
changes against 1.8.12, then ported them to the tip of the CVS tree.
I didn't see that this was already fixed.
thanks
dan
No worries.
Z.
Post by Dan Gora
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?
thanks
dan
--
Dan Gora
Software Engineer

Adax, Inc.
Av Dona Maria Alves, 1070 Casa 5
Centro
Ubatuba, SP
CEP 11680-000
Brasil

Tel: +55 (12) 3833-1021 (Brazil and outside of US)
: +1 (510) 859-4801 (Inside of US)
: dan_gora (Skype)

email: ***@adax.com
Zdenek Styblik
2013-04-25 07:55:33 UTC
Permalink
Post by Dan Gora
Here's a quick patch to fix this.
thanks
dan
Hello Dan,

you're absolutely correct. I've checked other files whether there is
some better way, but it seems 'bswap.h' gets included unconditionally
even though it's not needed at all in some cases.
Despite I wanted to mess around with 'config.h' and #ifdef-s, but it
doesn't seem to be worth of the time spent given the previous search.

It's committed now and hopefully fixed for good.

Thanks,
Z.
Post by Dan Gora
commit b1f38c34edc679eac03613b6ceb5825deca2e854
Date: Wed Apr 24 19:09:54 2013 -0300
[debian] ipmitool: Fix compilation bug on big endian machines.
On big endian machines the compilation would fail because BSWAP_32
was not defined in ipmi_chassis.c and ipmi_pef.c.
The problem was including bswap.h depended on WORDS_BIGENDIAN which
comes from <config.h>, however none of the previous header files
would include config.h, so it remained undefined.
We now just include the bswap.h unconditionally.
diff --git a/linux/debian/ipmitool/lib/ipmi_chassis.c
b/linux/debian/ipmitool/lib/ipmi_chassis.c
index 9f71d32..aaf3878 100644
--- a/linux/debian/ipmitool/lib/ipmi_chassis.c
+++ b/linux/debian/ipmitool/lib/ipmi_chassis.c
@@ -35,10 +35,7 @@
#include <stdio.h>
#include <time.h>
-#if WORDS_BIGENDIAN
-# include <ipmitool/bswap.h>
-#endif
-
+#include <ipmitool/bswap.h>
#include <ipmitool/helper.h>
#include <ipmitool/ipmi.h>
#include <ipmitool/log.h>
diff --git a/linux/debian/ipmitool/lib/ipmi_pef.c
b/linux/debian/ipmitool/lib/ipmi_pef.c
index c0fa00d..154bf40 100644
--- a/linux/debian/ipmitool/lib/ipmi_pef.c
+++ b/linux/debian/ipmitool/lib/ipmi_pef.c
@@ -34,10 +34,7 @@
#include <math.h>
#include <time.h>
-#if WORDS_BIGENDIAN
-# include <ipmitool/bswap.h>
-#endif
-
+#include <ipmitool/bswap.h>
#include <ipmitool/helper.h>
#include <ipmitool/log.h>
#include <ipmitool/ipmi.h>
Post by Dan Gora
Hi Zdenek,
It turns out that this bug is not a duplicate after all.. The TOB of
the CVS tree as of today still has the same compilation failure on big
endian machines.
The problem is the include files in ipmi_pef.c and ipmi_chassis.c
#include <string.h>
#include <math.h>
#include <time.h>
#if WORDS_BIGENDIAN
# include <ipmitool/bswap.h>
#endif
#include <ipmitool/helper.h>
#include <ipmitool/log.h>
#include <ipmitool/ipmi.h>
#include <ipmitool/ipmi_intf.h>
#include <ipmitool/ipmi_pef.h>
In this case, bswap.h doesn't get included because there are no other
header files before it which include <config.h>, so WORDS_BIGENDIAN is
not defined until ipmi.h is included.
The easiest solution is just to remove the #if WORDS_BIGENDIAN from
around the #include <ipmitool/bswap.h>.
thanks
dan
Post by Zdenek Styblik
Post by Dan Gora
Ok, never mind.. it was.. Sorry about that.. I originally did all the
changes against 1.8.12, then ported them to the tip of the CVS tree.
I didn't see that this was already fixed.
thanks
dan
No worries.
Z.
Post by Dan Gora
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
The BSWAP_32 macro was added for big endian machines in these files,
but the author forgot to include the necessary header file which
defines it.
This is already fixed.
This was after the 1.8.12 release, right?
thanks
dan
--
Dan Gora
Software Engineer
Adax, Inc.
Av Dona Maria Alves, 1070 Casa 5
Centro
Ubatuba, SP
CEP 11680-000
Brasil
Tel: +55 (12) 3833-1021 (Brazil and outside of US)
: +1 (510) 859-4801 (Inside of US)
: dan_gora (Skype)
Loading...