Skip to contentSkip to conversation

e2fsprogs: use ALL_CFLAGS to build gen_crc32ctable

PATCH In review Eric Sandeen 2 participants 5 messages Oct 3, 2011 – Oct 10, 2011

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 e2fsprogs: use ALL_CFLAGS to build gen_crc32ctable

With just BUILD_CFLAGS this we don't get the right include paths (i.e. within the tree) to build the gen_crc32ctable binary.

---

I think that the rule could probably be removed altogether, and gen_crc32ctable.o would be built from gen_crc32ctable.c, and the normal dependencies & rules would build gen_crc32ctable. But this seems like the most obvious change.

lib/ext2fs/Makefile.in +1 -1
index 3de9f87..07a0db18 100644
@@ -413,7 +413,7 @@ $(OBJS): subdirs
 
 gen_crc32ctable: $(srcdir)/gen_crc32ctable.c
 	$(E) "	CC $@"
-	$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32ctable \
+	$(Q) $(BUILD_CC) $(ALL_CFLAGS) -o gen_crc32ctable \
 		$(srcdir)/gen_crc32ctable.c
 
 crc32c_table.h: gen_crc32ctable
#1 lore raw
Quoted 4 lines · Eric Sandeen

With just BUILD_CFLAGS this we don't get the right include paths (i.e. within the tree) to build the gen_crc32ctable binary.

We can't use ALL_CFLAGS because in the cross compilation case, it might contain some compiler directives that make sense for the target platform, but not for the host platform. BUILD_CFLAGS is specifically designed for this case. For programs like gen_crc32ctable that need to compiled and run on the host OS, you have to do the following so the right thing happens when you are cross-compiling e2fsprogs (yes, there are people who do this!):

gen_crc32ctable: $(srcdir)/gen_crc32ctable.c

	$(E) "	CC $@"
	$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32ctable \
		$(srcdir)/gen_crc32ctable.c

Note the use of $(BUILD_CC) instead of $(CC).

The better fix is to note that gen_crc32ctable doesn't actually need anything from ext2fs.h, and move that #include to crc32c.c.

- Ted

commit 8232f2ddaece256afd323eb5a289a5ec1b7fbd1a

Author: Theodore Ts'o <[email protected]>
Date:   Mon Oct 3 22:49:45 2011 -0400

libext2fs: move #include "ext2fs.h" from crc32c_defs.h to crc32c.c

The byte swap functions which are defined in ext2fs.h are only needed by crc32.c, and not by gen_crc32ctable.c. The gen_crc32ctable program needs to be compiled on the host OS, where ext2fs.h may not be present. So move the use of the header function to crc32c.c, to avoid compilation problems.

lib/ext2fs/crc32c.c +17 -0
index 65bca5c..6be4336 100644
@@ -40,6 +40,23 @@
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #include "crc32c_defs.h"
 
+#include "ext2fs.h"
+#ifdef WORDS_BIGENDIAN
+#define __constant_cpu_to_le32(x) ___constant_swab32((x))
+#define __constant_cpu_to_be32(x) (x)
+#define __be32_to_cpu(x) (x)
+#define __cpu_to_be32(x) (x)
+#define __cpu_to_le32(x) (ext2fs_cpu_to_le32((x)))
+#define __le32_to_cpu(x) (ext2fs_le32_to_cpu((x)))
+#else
+#define __constant_cpu_to_le32(x) (x)
+#define __constant_cpu_to_be32(x) ___constant_swab32((x))
+#define __be32_to_cpu(x) (ext2fs_be32_to_cpu((x)))
+#define __cpu_to_be32(x) (ext2fs_cpu_to_be32((x)))
+#define __cpu_to_le32(x) (x)
+#define __le32_to_cpu(x) (x)
+#endif
+
 #if CRC_LE_BITS > 8
 # define tole(x) (__force uint32_t) __constant_cpu_to_le32(x)
 #else
lib/ext2fs/crc32c_defs.h +0 -16
index ced4f67..023f2c0 100644
@@ -42,23 +42,6 @@
 		(((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24)))
 
 
-#include "ext2fs.h"
-#ifdef WORDS_BIGENDIAN
-#define __constant_cpu_to_le32(x) ___constant_swab32((x))
-#define __constant_cpu_to_be32(x) (x)
-#define __be32_to_cpu(x) (x)
-#define __cpu_to_be32(x) (x)
-#define __cpu_to_le32(x) (ext2fs_cpu_to_le32((x)))
-#define __le32_to_cpu(x) (ext2fs_le32_to_cpu((x)))
-#else
-#define __constant_cpu_to_le32(x) (x)
-#define __constant_cpu_to_be32(x) ___constant_swab32((x))
-#define __be32_to_cpu(x) (ext2fs_be32_to_cpu((x)))
-#define __cpu_to_be32(x) (ext2fs_cpu_to_be32((x)))
-#define __cpu_to_le32(x) (x)
-#define __le32_to_cpu(x) (x)
-#endif
#2 lore raw
Quoted 45 lines · Ted Ts'o
Quoted 4 lines · Eric Sandeen

With just BUILD_CFLAGS this we don't get the right include paths (i.e. within the tree) to build the gen_crc32ctable binary.

We can't use ALL_CFLAGS because in the cross compilation case, it might contain some compiler directives that make sense for the target platform, but not for the host platform. BUILD_CFLAGS is specifically designed for this case. For programs like gen_crc32ctable that need to compiled and run on the host OS, you have to do the following so the right thing happens when you are cross-compiling e2fsprogs (yes, there are people who do this!):

gen_crc32ctable: $(srcdir)/gen_crc32ctable.c

	$(E) "	CC $@"
	$(Q) $(BUILD_CC) $(BUILD_CFLAGS) -o gen_crc32ctable \
		$(srcdir)/gen_crc32ctable.c

Note the use of $(BUILD_CC) instead of $(CC).

The better fix is to note that gen_crc32ctable doesn't actually need anything from ext2fs.h, and move that #include to crc32c.c.

- Ted

commit 8232f2ddaece256afd323eb5a289a5ec1b7fbd1a

Author: Theodore Ts'o <[email protected]>
Date:   Mon Oct 3 22:49:45 2011 -0400

libext2fs: move #include "ext2fs.h" from crc32c_defs.h to crc32c.c

The byte swap functions which are defined in ext2fs.h are only needed by crc32.c, and not by gen_crc32ctable.c. The gen_crc32ctable program needs to be compiled on the host OS, where ext2fs.h may not be present. So move the use of the header function to crc32c.c, to avoid compilation problems.

lib/ext2fs/crc32c.c +0 -0
index 65bca5c..6be4336 100644

lib/ext2fs/crc32c.c line 44

Quoted 5 lines
lib/ext2fs/crc32c.c +2 -0
@@ -40,6 +40,23 @@
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #include "crc32c_defs.h"
 
+#include "ext2fs.h"
+#ifdef WORDS_BIGENDIAN

well, this has the exact same problem as originally reported, because we still don't have the include paths to find ext2fs.h in the tree, and it is still included via crc32c_defs.h

Perhaps I should have been more explicit about it last time.

I get:

making all in lib/ext2fs
make[2]: Entering directory `/src2/git/fedora/e2fsprogs/e2fsprogs-1.42/lib/ext2fs'

gcc -g -O2 -o gen_crc32ctable \
		./gen_crc32ctable.c
In file included from ./crc32c_defs.h:45,
                 from ./gen_crc32ctable.c:2:

./ext2fs.h:73:31: error: ext2fs/ext2_types.h: No such file or directory ./ext2fs.h:74:28: error: ext2fs/ext2_fs.h: No such file or directory ./ext2fs.h:75:33: error: ext2fs/ext3_extents.h: No such file or directory In file included from ./crc32c_defs.h:45,

                 from ./gen_crc32ctable.c:2:

./ext2fs.h:78: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘ext2_ino_t’

...

Looking into it, but if you have an obvious solution I'm all ears.

-Eric

Quoted 47 lines
lib/ext2fs/crc32c.c +15 -0
+#define __constant_cpu_to_le32(x) ___constant_swab32((x))
+#define __constant_cpu_to_be32(x) (x)
+#define __be32_to_cpu(x) (x)
+#define __cpu_to_be32(x) (x)
+#define __cpu_to_le32(x) (ext2fs_cpu_to_le32((x)))
+#define __le32_to_cpu(x) (ext2fs_le32_to_cpu((x)))
+#else
+#define __constant_cpu_to_le32(x) (x)
+#define __constant_cpu_to_be32(x) ___constant_swab32((x))
+#define __be32_to_cpu(x) (ext2fs_be32_to_cpu((x)))
+#define __cpu_to_be32(x) (ext2fs_cpu_to_be32((x)))
+#define __cpu_to_le32(x) (x)
+#define __le32_to_cpu(x) (x)
+#endif
+
 #if CRC_LE_BITS > 8
 # define tole(x) (__force uint32_t) __constant_cpu_to_le32(x)
 #else
lib/ext2fs/crc32c_defs.h +0 -18
index ced4f67..023f2c0 100644
@@ -42,23 +42,6 @@
 		(((uint32_t)(x) & (uint32_t)0xff000000UL) >> 24)))
 
 
-#include "ext2fs.h"
-#ifdef WORDS_BIGENDIAN
-#define __constant_cpu_to_le32(x) ___constant_swab32((x))
-#define __constant_cpu_to_be32(x) (x)
-#define __be32_to_cpu(x) (x)
-#define __cpu_to_be32(x) (x)
-#define __cpu_to_le32(x) (ext2fs_cpu_to_le32((x)))
-#define __le32_to_cpu(x) (ext2fs_le32_to_cpu((x)))
-#else
-#define __constant_cpu_to_le32(x) (x)
-#define __constant_cpu_to_be32(x) ___constant_swab32((x))
-#define __be32_to_cpu(x) (ext2fs_be32_to_cpu((x)))
-#define __cpu_to_be32(x) (ext2fs_cpu_to_be32((x)))
-#define __cpu_to_le32(x) (x)
-#define __le32_to_cpu(x) (x)
-#endif
-
 #if (__GNUC__ >= 3)
 #define likely(x)	__builtin_expect(!!(x), 1)
 #define unlikely(x)	__builtin_expect(!!(x), 0)
--
Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 1 line · Eric Sandeen

On 10/3/11 10:02 PM, Ted Ts'o wrote:

...

Quoted 8 lines
Quoted 8 lines
lib/ext2fs/crc32c.c +0 -0
index 65bca5c..6be4336 100644
@@ -40,6 +40,23 @@
 #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
 #include "crc32c_defs.h"
 
Quoted 6 lines

lib/ext2fs/crc32c.c line 44

Quoted 2 lines
lib/ext2fs/crc32c.c +2 -0
+#include "ext2fs.h"
+#ifdef WORDS_BIGENDIAN

well, this has the exact same problem as originally reported, because we still don't have the include paths to find ext2fs.h in the tree, and it is still included via crc32c_defs.h

Sorry, ignore that; PEBKAC here. :(

(tarball naming convention change means I had to fix up the source tarball name in 2 places in the spec, and I missed one. Apologies...)

Now I am getting translation table problems, but I'll see what's up there.

mv: cannot stat `t-es.gmo': No such file or directory
mv: cannot stat `t-ca.gmo': No such file or directorymake[2]: *** [es.gmo] Error 1

-Eric

#4 lore raw
Quoted 4 lines · Eric Sandeen

mv: cannot stat `t-es.gmo': No such file or directory
mv: cannot stat `t-ca.gmo': No such file or directorymake[2]: *** [es.gmo] Error 1

-Eric

Ok, I needed gettext in the build root now.

It should probably fail more gracefully, but I won't worry about that for now.

I also noticed that configure didn't really handle the missing msgfmt:

checking for msgfmt... no
checking for gmsgfmt... :

I'll take a quick look at that.

Anyway, latest WIP release is finally in Rawhide. :)

-Eric