diff mbox series

e2fsprogs: clean up architecture dependent header files

Message ID 20180821145956.16827-1-lczerner@redhat.com
State New, archived
Headers show
Series e2fsprogs: clean up architecture dependent header files | expand

Commit Message

Lukas Czerner Aug. 21, 2018, 2:59 p.m. UTC 
Currently blkid_types.h and ext2_types.h header files are architecture
dependent which is messy and creates problems when building packages for
multilib architectures. The API should really ideally be architecture
independent.

Since we already include stdint.h use the exact-width integer types to
define the specific __[u,s][8,16,32] types which is easy to do. In
future we might want to get rid of this entirely and just use stdint.h
types directly.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
NOTE: Outside of couple of linux distributions I did test this only on
freebsd where it seems to be working fine.

 lib/blkid/blkid_types.h.in | 165 ++----------------------------
 lib/ext2fs/ext2_types.h.in | 203 +++++--------------------------------
 misc/e2freefrag.c          |   8 +-
 3 files changed, 42 insertions(+), 334 deletions(-)

Comments

Theodore Ts'o Aug. 21, 2018, 4:07 p.m. UTC | #1 
On Tue, Aug 21, 2018 at 04:59:56PM +0200, Lukas Czerner wrote:
> Currently blkid_types.h and ext2_types.h header files are architecture
> dependent which is messy and creates problems when building packages for
> multilib architectures. The API should really ideally be architecture
> independent.
> 
> Since we already include stdint.h use the exact-width integer types to
> define the specific __[u,s][8,16,32] types which is easy to do. In
> future we might want to get rid of this entirely and just use stdint.h
> types directly.

We were *supposed* to have only used stdint.h if it exists:

#ifdef HAVE_STDINT_H
#include <stdint.h>
#endif

However, it looks like we've gotten sloppy and there are places such as
lib/ext2fs/crc32c.c where we don't protect the stdint inclusion and
used stdint.h types --- and no one has complained.

In particular, the last holdout I'm aware of where this was a problem,
MSVC (Microsoft Visual Studio's C compiler) is apparently OK now.
(MSVC only got around to adding stdint.h *TEN YEARS* after C99 was
released, in 2010.  And anyone who hasn't upgraded to MSVC 2010 or
later in 2018 gets zero sympathy from me.  :-)

So sure, we can try using stdint.h, and hopefully nothing will break.
But the main reason why I'm interested is build simplification.
Specifically, see if we can get rid of all uses of HAVE_STDINT_H and
then get rid of the stdint.h test in configure.ac.  Even better, this
should also allow us to stop making *_types.h generated files, which
means they should be renamed to *_types.h.  This will allow us to get
rid of config/parse_types.sh, and the Makefile.in's which used
parse_types.sh.

Would you be interested in taking a look at that cleanup?

		      	      	       	     - Ted

P.S.  I'm curious why you had move the #include's around in e2freefrag.c?
diff mbox series

Patch

diff --git a/lib/blkid/blkid_types.h.in b/lib/blkid/blkid_types.h.in
index d4c81d01..758edff5 100644
--- a/lib/blkid/blkid_types.h.in
+++ b/lib/blkid/blkid_types.h.in
@@ -3,165 +3,20 @@ 
  * everything we need.  (cross fingers)  Other header files may have 
  * also defined the types that we need.
  */
+#include <stdint.h>
+
 #if (!defined(_LINUX_TYPES_H) && !defined(_BLKID_TYPES_H) && \
 	!defined(_EXT2_TYPES_H))
 #define _BLKID_TYPES_H
 
-@ASM_TYPES_HEADER@
-
-#ifndef HAVE___U8
-#define HAVE___U8
-#ifdef __U8_TYPEDEF
-typedef __U8_TYPEDEF __u8;
-#else
-typedef unsigned char __u8;
-#endif
-#endif /* HAVE___U8 */
-
-#ifndef HAVE___S8
-#define HAVE___S8
-#ifdef __S8_TYPEDEF
-typedef __S8_TYPEDEF __s8;
-#else
-typedef signed char __s8;
-#endif
-#endif /* HAVE___S8 */
-
-#ifndef HAVE___U16
-#define HAVE___U16
-#ifdef __U16_TYPEDEF
-typedef __U16_TYPEDEF __u16;
-#else
-#if (@SIZEOF_INT@ == 2)
-typedef	unsigned int	__u16;
-#else
-#if (@SIZEOF_SHORT@ == 2)
-typedef	unsigned short	__u16;
-#else
-#undef HAVE___U16
-  ?==error: undefined 16 bit type
-#endif /* SIZEOF_SHORT == 2 */
-#endif /* SIZEOF_INT == 2 */
-#endif /* __U16_TYPEDEF */
-#endif /* HAVE___U16 */
-
-#ifndef HAVE___S16
-#define HAVE___S16
-#ifdef __S16_TYPEDEF
-typedef __S16_TYPEDEF __s16;
-#else
-#if (@SIZEOF_INT@ == 2)
-typedef	int		__s16;
-#else
-#if (@SIZEOF_SHORT@ == 2)
-typedef	short		__s16;
-#else
-#undef HAVE___S16
-  ?==error: undefined 16 bit type
-#endif /* SIZEOF_SHORT == 2 */
-#endif /* SIZEOF_INT == 2 */
-#endif /* __S16_TYPEDEF */
-#endif /* HAVE___S16 */
-
-#ifndef HAVE___U32
-#define HAVE___U32
-#ifdef __U32_TYPEDEF
-typedef __U32_TYPEDEF __u32;
-#else
-#if (@SIZEOF_INT@ == 4)
-typedef	unsigned int	__u32;
-#else
-#if (@SIZEOF_LONG@ == 4)
-typedef	unsigned long	__u32;
-#else
-#if (@SIZEOF_SHORT@ == 4)
-typedef	unsigned short	__u32;
-#else
-#undef HAVE___U32
- ?== error: undefined 32 bit type
-#endif /* SIZEOF_SHORT == 4 */
-#endif /* SIZEOF_LONG == 4 */
-#endif /* SIZEOF_INT == 4 */
-#endif /* __U32_TYPEDEF */
-#endif /* HAVE___U32 */
-
-#ifndef HAVE___S32
-#define HAVE___S32
-#ifdef __S32_TYPEDEF
-typedef __S32_TYPEDEF __s32;
-#else
-#if (@SIZEOF_INT@ == 4)
-typedef	int		__s32;
-#else
-#if (@SIZEOF_LONG@ == 4)
-typedef	long		__s32;
-#else
-#if (@SIZEOF_SHORT@ == 4)
-typedef	short		__s32;
-#else
-#undef HAVE___S32
- ?== error: undefined 32 bit type
-#endif /* SIZEOF_SHORT == 4 */
-#endif /* SIZEOF_LONG == 4 */
-#endif /* SIZEOF_INT == 4 */
-#endif /* __S32_TYPEDEF */
-#endif /* HAVE___S32 */
-
-#ifndef HAVE___U64
-#define HAVE___U64
-#ifdef __U64_TYPEDEF
-typedef __U64_TYPEDEF __u64;
-#else
-#if (@SIZEOF_INT@ == 8)
-typedef unsigned int	__u64;
-#else
-#if (@SIZEOF_LONG_LONG@ == 8)
-typedef unsigned long long	__u64;
-#else
-#if (@SIZEOF_LONG@ == 8)
-typedef unsigned long	__u64;
-#else
-#undef HAVE___U64
- ?== error: undefined 64 bit type
-#endif /* SIZEOF_LONG == 8 */
-#endif /* SIZEOF_LONG_LONG == 8 */
-#endif /* SIZEOF_INT == 8 */
-#endif /* __U64_TYPEDEF */
-#endif /* HAVE___U64 */
-
-#ifndef HAVE___S64
-#define HAVE___S64
-#ifdef __S64_TYPEDEF
-typedef __S64_TYPEDEF __s64;
-#else
-#if (@SIZEOF_INT@ == 8)
-typedef int		__s64;
-#else
-#if (@SIZEOF_LONG_LONG@ == 8)
-#if defined(__GNUC__)
-typedef __signed__ long long	__s64;
-#else
-typedef signed long long	__s64;
-#endif /* __GNUC__ */
-#else
-#if (@SIZEOF_LONG@ == 8)
-typedef long		__s64;
-#else
-#undef HAVE___S64
- ?== error: undefined 64 bit type
-#endif /* SIZEOF_LONG == 8 */
-#endif /* SIZEOF_LONG_LONG == 8 */
-#endif /* SIZEOF_INT == 8 */
-#endif /* __S64_TYPEDEF */
-#endif /* HAVE___S64 */
+typedef		int8_t		__s8;
+typedef		int16_t		__s16;
+typedef		int32_t		__s32;
+typedef		int64_t		__s64;
 
-#undef __S8_TYPEDEF
-#undef __U8_TYPEDEF
-#undef __S16_TYPEDEF
-#undef __U16_TYPEDEF
-#undef __S32_TYPEDEF
-#undef __U32_TYPEDEF
-#undef __S64_TYPEDEF
-#undef __U64_TYPEDEF
+typedef		uint8_t		__u8;
+typedef		uint16_t	__u16;
+typedef		uint32_t	__u32;
+typedef		uint64_t	__u64;
 
 #endif /* _*_TYPES_H */
diff --git a/lib/ext2fs/ext2_types.h.in b/lib/ext2fs/ext2_types.h.in
index 98cc65b4..9b1ccdbc 100644
--- a/lib/ext2fs/ext2_types.h.in
+++ b/lib/ext2fs/ext2_types.h.in
@@ -3,194 +3,47 @@ 
  * everything we need.  (cross fingers)  Other header files may have
  * also defined the types that we need.
  */
-#if (!defined(_LINUX_TYPES_H) && !defined(_BLKID_TYPES_H) && \
-	!defined(_EXT2_TYPES_H))
-#define _EXT2_TYPES_H
+#include <stdint.h>
 
-@ASM_TYPES_HEADER@
+#if !defined(_EXT2_TYPES_H)
+#if !defined(_LINUX_TYPES_H)
+#if !defined(_BLKID_TYPES_H)
 
-#ifndef HAVE___U8
-#define HAVE___U8
-#ifdef __U8_TYPEDEF
-typedef __U8_TYPEDEF __u8;
-#else
-typedef unsigned char __u8;
-#endif
-#endif /* HAVE___U8 */
+typedef		int8_t		__s8;
+typedef		int16_t		__s16;
+typedef		int32_t		__s32;
+typedef		int64_t		__s64;
 
-#ifndef HAVE___S8
-#define HAVE___S8
-#ifdef __S8_TYPEDEF
-typedef __S8_TYPEDEF __s8;
-#else
-typedef signed char __s8;
-#endif
-#endif /* HAVE___S8 */
+typedef		uint8_t		__u8;
+typedef		uint16_t	__u16;
+typedef		uint32_t	__u32;
+typedef		uint64_t	__u64;
 
-#ifndef HAVE___U16
-#define HAVE___U16
-#ifdef __U16_TYPEDEF
-typedef __U16_TYPEDEF __u16;
-#else
-#if (@SIZEOF_INT@ == 2)
-typedef	unsigned int	__u16;
-#else
-#if (@SIZEOF_SHORT@ == 2)
-typedef	unsigned short	__u16;
-#else
-#undef HAVE___U16
-  ?==error: undefined 16 bit type
-#endif /* SIZEOF_SHORT == 2 */
-#endif /* SIZEOF_INT == 2 */
-#endif /* __U16_TYPEDEF */
-#endif /* HAVE___U16 */
+#endif /* _BLKID_TYPES_H */
 
-#ifndef HAVE___S16
-#define HAVE___S16
-#ifdef __S16_TYPEDEF
-typedef __S16_TYPEDEF __s16;
-#else
-#if (@SIZEOF_INT@ == 2)
-typedef	int		__s16;
-#else
-#if (@SIZEOF_SHORT@ == 2)
-typedef	short		__s16;
-#else
-#undef HAVE___S16
-  ?==error: undefined 16 bit type
-#endif /* SIZEOF_SHORT == 2 */
-#endif /* SIZEOF_INT == 2 */
-#endif /* __S16_TYPEDEF */
-#endif /* HAVE___S16 */
-
-#ifndef HAVE___U32
-#define HAVE___U32
-#ifdef __U32_TYPEDEF
-typedef __U32_TYPEDEF __u32;
-#else
-#if (@SIZEOF_INT@ == 4)
-typedef	unsigned int	__u32;
-#else
-#if (@SIZEOF_LONG@ == 4)
-typedef	unsigned long	__u32;
-#else
-#if (@SIZEOF_SHORT@ == 4)
-typedef	unsigned short	__u32;
-#else
-#undef HAVE___U32
- ?== error: undefined 32 bit type
-#endif /* SIZEOF_SHORT == 4 */
-#endif /* SIZEOF_LONG == 4 */
-#endif /* SIZEOF_INT == 4 */
-#endif /* __U32_TYPEDEF */
-#endif /* HAVE___U32 */
-
-#ifndef HAVE___S32
-#define HAVE___S32
-#ifdef __S32_TYPEDEF
-typedef __S32_TYPEDEF __s32;
-#else
-#if (@SIZEOF_INT@ == 4)
-typedef	int		__s32;
-#else
-#if (@SIZEOF_LONG@ == 4)
-typedef	long		__s32;
-#else
-#if (@SIZEOF_SHORT@ == 4)
-typedef	short		__s32;
-#else
-#undef HAVE___S32
- ?== error: undefined 32 bit type
-#endif /* SIZEOF_SHORT == 4 */
-#endif /* SIZEOF_LONG == 4 */
-#endif /* SIZEOF_INT == 4 */
-#endif /* __S32_TYPEDEF */
-#endif /* HAVE___S32 */
-
-#ifndef HAVE___U64
-#define HAVE___U64
-#ifdef __U64_TYPEDEF
-typedef __U64_TYPEDEF __u64;
-#else
-#if (@SIZEOF_INT@ == 8)
-typedef unsigned int	__u64;
-#else
-#if (@SIZEOF_LONG_LONG@ == 8)
-typedef unsigned long long	__u64;
-#else
-#if (@SIZEOF_LONG@ == 8)
-typedef unsigned long	__u64;
-#else
-#undef HAVE___U64
- ?== error: undefined 64 bit type
-#endif /* SIZEOF_LONG_LONG == 8 */
-#endif /* SIZEOF_LONG == 8 */
-#endif /* SIZEOF_INT == 8 */
-#endif /* __U64_TYPEDEF */
-#endif /* HAVE___U64 */
-
-#ifndef HAVE___S64
-#define HAVE___S64
-#ifdef __S64_TYPEDEF
-typedef __S64_TYPEDEF __s64;
-#else
-#if (@SIZEOF_INT@ == 8)
-typedef int		__s64;
-#else
-#if (@SIZEOF_LONG_LONG@ == 8)
-#if defined(__GNUC__)
-typedef __signed__ long long	__s64;
-#else
-typedef signed long long	__s64;
-#endif /* __GNUC__ */
-#else
-#if (@SIZEOF_LONG@ == 8)
-typedef long		__s64;
+#ifdef __CHECKER__
+#define __bitwise__ __attribute__((bitwise))
 #else
-#undef HAVE___S64
- ?== error: undefined 64 bit type
-#endif /* SIZEOF_LONG_LONG == 8 */
-#endif /* SIZEOF_LONG == 8 */
-#endif /* SIZEOF_INT == 8 */
-#endif /* __S64_TYPEDEF */
-#endif /* HAVE___S64 */
-
-#undef __S8_TYPEDEF
-#undef __U8_TYPEDEF
-#undef __S16_TYPEDEF
-#undef __U16_TYPEDEF
-#undef __S32_TYPEDEF
-#undef __U32_TYPEDEF
-#undef __S64_TYPEDEF
-#undef __U64_TYPEDEF
+#define __bitwise__
+#endif
+#define __bitwise __bitwise__
 
-#endif /* _*_TYPES_H */
+typedef __u16  __bitwise       __le16;
+typedef __u32  __bitwise       __le32;
+typedef __u64  __bitwise       __le64;
+typedef __u16  __bitwise       __be16;
+typedef __u32  __bitwise       __be32;
+typedef __u64  __bitwise       __be64;
 
-#include <stdint.h>
+#endif /* _LINUX_TYPES_H */
 
-/* endian checking stuff */
-#ifndef EXT2_ENDIAN_H_
-#define EXT2_ENDIAN_H_
+@PUBLIC_CONFIG_HEADER@
 
 #ifdef __CHECKER__
-# ifndef __bitwise
-#  define __bitwise		__attribute__((bitwise))
-# endif
 #define __force			__attribute__((force))
 #else
-# ifndef __bitwise
-#  define __bitwise
-# endif
 #define __force
 #endif
 
-typedef __u16	__bitwise	__le16;
-typedef __u32	__bitwise	__le32;
-typedef __u64	__bitwise	__le64;
-typedef __u16	__bitwise	__be16;
-typedef __u32	__bitwise	__be32;
-typedef __u64	__bitwise	__be64;
-
-#endif /* EXT2_ENDIAN_H_ */
-
-@PUBLIC_CONFIG_HEADER@
+#define _EXT2_TYPES_H
+#endif /* _EXT2_TYPES_H */
diff --git a/misc/e2freefrag.c b/misc/e2freefrag.c
index 6eb4c736..83fd7fb6 100644
--- a/misc/e2freefrag.c
+++ b/misc/e2freefrag.c
@@ -33,10 +33,6 @@  extern int optind;
 # include <limits.h>
 #endif
 
-#include "ext2fs/ext2_fs.h"
-#include "ext2fs/ext2fs.h"
-#include "e2freefrag.h"
-
 #if defined(HAVE_EXT2_IOCTLS) && !defined(DEBUGFS)
 # ifdef HAVE_LINUX_FSMAP_H
 #  include <linux/fsmap.h>
@@ -44,6 +40,10 @@  extern int optind;
 # include "fsmap.h"
 #endif
 
+#include "ext2fs/ext2_fs.h"
+#include "ext2fs/ext2fs.h"
+#include "e2freefrag.h"
+
 #ifndef PATH_MAX
 #define PATH_MAX 4096
 #endif