u_int32_t usage in rpmdb
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
RPM |
Invalid
|
Medium
|
Jeff Johnson |
Bug Description
Building rpm-5.4.12 on x86_64-
Whether you're talking about Sun's Solaris 10, Oracle Solaris 11, or any of the OpenSolaris derivatives, none of the system headers provide the BSD-compatible sized unsigned typedefs (e.g. u_int8_t, u_int16_t, u_int32_t, etc.). Instead, Solaris provides the names that C99 standardized on, uint8_t, uint16_t, uint32_t, etc.
There are at least a couple places in rpmdb.c and rpmtxn.c where u_int32_t is used in a cast. That causes build failures since that typedef is not present on Solaris:
source='rpmdb.c' object=
/bin/sh ../libtool --tag=CC --mode=compile cc -m64 -DHAVE_CONFIG_H -I. -I.. -I. -I.. -I../build -I../lib -I../lib -I../rpmdb -I../rpmio -I../misc -I../beecrypt/
libtool: compile: cc -m64 -DHAVE_CONFIG_H -I. -I.. -I. -I.. -I../build -I../lib -I../lib -I../rpmdb -I../rpmio -I../misc -I../beecrypt/
"./rpmtag.h", line 107: warning: enumerator value overflows INT_MAX (2147483647)
"./rpmtag.h", line 440: warning: enumerator value overflows INT_MAX (2147483647)
"rpmdb.c", line 274: warning: implicit function declaration: db3Free
"rpmdb.c", line 274: warning: improper pointer/integer combination: op "="
"rpmdb.c", line 1775: undefined symbol: u_int32_t
"rpmdb.c", line 1775: syntax error before or at: mi
"rpmdb.c", line 2223: undefined symbol: u_int32_t
"rpmdb.c", line 2223: syntax error before or at: uhlen
"rpmdb.c", line 2289: undefined symbol: u_int32_t
"rpmdb.c", line 2289: syntax error before or at: mi
cc: acomp failed for rpmdb.c
gmake[2]: *** [librpmdb_
gmake[2]: Leaving directory `/local/
I see nearby places with casts that instead use the available define UINT32_T, so switching to that would be one possible fix for rpmdb.c, but rpmtxn.c also has the issue and it doesn't provide the same typedef.
Another possible fix would be to use rpmuint32_t, which also seems to be widely used throughout the code.
A third possible fix would be to augment this code (from the top of rpmdb.c):
/* XXX retrofit the *BSD typedef for the deprived. */
#if defined(__QNXNTO__)
typedef rpmuint32_t u_int32_t;
#endif
to support more platforms.
A fourth possibility would be to switch everything to use the typedef names that C99 standardized.
I'm happy to provide a patch for any of these, just let me know which one you prefer. The default approach I took with the attached patch was to use rpmuint32_t, since that's available via inclusion for both rpmdb.c and rpmtxn.c.
While hunting for the origin of rpmuint32_t, I also noted that rpmio/rpmiotypes.h uses a typedef of "unsigned long long" for rpmuint64_t. That currently works on Solaris even when building in 64 bit mode (which I generally do), but I have a vague recollection of reading that in the future, "unsigned long long" may become a 16 byte quantity when building in LP64 mode.
It might be better to protect that typedef with an "#ifdef _LP64" guard, and use just "unsigned long" for rpmuint64_t in that case. I haven't done that and it may not even be advisable, though I don't see how it could hurt.
Let me know if you would prefer a patch that chooses one of the other options. I dislike mixing UINT32_T casts and rpmuint32_t casts in rpmdb.c in such close proximity, so more work to switch to one or the other seems like a good idea
to me. Just let me know what your preference is.
Changed in rpm: | |
importance: | Undecided → Medium |
milestone: | none → 5.4.13 |
assignee: | nobody → Jeff Johnson (n3npq) |
status: | New → Confirmed |
Changed in rpm: | |
status: | Confirmed → Invalid |
milestone: | 5.4.13 → 5.4.14 |
Hmmm … Berkeley DB retrofits these typedefs if/when not
available.
While no. of bits and signedness (and promotion and sign extension …) coverity/ clang
is all that matters to the CPU, various static analyzers like splint/
actually generate lots of sewage if not used carefully.
At least on my CenttOS 6.4 box and Mac OS X Lion, u_int32_t et al are
definitely in use throughout the Berkeley DB API.
The rpmuint32_t is used for integers retrieved from Header metadata,
my older practice is/was to always use unsigned/int internally to RPM,
but all the cool linux puppies tell me about
#include <stdint.h>
once or twice a year.
So the patch should likely _NOT_ use rpmuint32_t unless uint32_t isn't
available. And I'm surprised that Berkeley DB did not retrofit either the
good old u_int32_t typedef's (what I see) or something equivalent.
What is needed?