RPM

u_int32_t usage in rpmdb

Bug #1220376 reported by Tim Mooney
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
RPM
Invalid
Medium
Jeff Johnson

Bug Description

Building rpm-5.4.12 on x86_64-pc-solaris2.11 (OpenIndiana oi151a8) with the Oracle Studio 12.3 compiler, though I think this particular issue would impact gcc builds on Solaris too.

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='librpmdb_la-rpmdb.lo' libtool=yes \
        DEPDIR=.deps depmode=none /bin/sh ../depcomp \
        /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/include -I../beecrypt/include -I../beecrypt -I../beecrypt -I../popt -I../popt -I../db -I../db -I../db/src -I../db/src -I../scripts -I../scripts -D__FUNCTION__=__func__ -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -DRPM_OS_SOLARIS=021100 -I/local/include -I/local/include/db60 -I/usr/include/pcre -D__FUNCTION__=__func__ -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -DRPM_OS_SOLARIS=021100 -I/local/include -I/local/include/db60 -I/usr/include/pcre -xopenmp -Xa -xc99=all -xO3 -g -mt -KPIC -D__FUNCTION__=__func__ -m64 -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -D_GNU_SOURCE -D_REENTRANT -c -o librpmdb_la-rpmdb.lo `test -f 'rpmdb.c' || echo './'`rpmdb.c
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/include -I../beecrypt/include -I../beecrypt -I../beecrypt -I../popt -I../popt -I../db -I../db -I../db/src -I../db/src -I../scripts -I../scripts -D__FUNCTION__=__func__ -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -DRPM_OS_SOLARIS=021100 -I/local/include -I/local/include/db60 -I/usr/include/pcre -D__FUNCTION__=__func__ -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -DRPM_OS_SOLARIS=021100 -I/local/include -I/local/include/db60 -I/usr/include/pcre -xopenmp -Xa -xc99=all -xO3 -g -mt -KPIC -D__FUNCTION__=__func__ -m64 -I/local/gnu/include -I/local/include -I/local/include/neon -I/local/include/beecrypt -I/local/include/2.7.5 -I/local/openssl/include -D_GNU_SOURCE -D_REENTRANT -c rpmdb.c -KPIC -DPIC -o .libs/librpmdb_la-rpmdb.o
"./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_la-rpmdb.lo] Error 1
gmake[2]: Leaving directory `/local/src/RPM/BUILD/rpm-5.4.12/rpmdb'

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.

Tags: solaris
Revision history for this message
Tim Mooney (tim-mooney) wrote :
Revision history for this message
Jeff Johnson (n3npq) wrote :

Hmmm … Berkeley DB retrofits these typedefs if/when not
available.

While no. of bits and signedness (and promotion and sign extension …)
is all that matters to the CPU, various static analyzers like splint/coverity/clang
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?

Revision history for this message
Jeff Johnson (n3npq) wrote :

There are some issues with tag data carried in an enum with
enforcing compilers.

All integers in metadata are unsigned since rpm-5.0. The easiest fix
is likely to add a cast in a handful of places where 0xFFFFFFFF (and
perhaps a few other convenience tags) are still in use.

Revision history for this message
Tim Mooney (tim-mooney) wrote :

There is a problem here, but it's triggered via a problem elsewhere. It looks like configure is not completely finding BerkeleyDB:

checking db.h usability... yes
checking db.h presence... yes
checking for db.h... yes
checking for db_create in -ldb-6.0... no
checking whether to build with Berkeley-DB library... no

Pre-processing rpmdb.c, I can see that "db.h" is never included, though db_emu.h is. So the u_int32_t is showing up because we're not getting BDB, we're getting db_emu.h from rpmdb.h.

I'll look into why configure isn't finding Berkeley DB and file a separate issue there, if it's an issue at all.

Jeff Johnson (n3npq)
Changed in rpm:
importance: Undecided → Medium
milestone: none → 5.4.13
assignee: nobody → Jeff Johnson (n3npq)
status: New → Confirmed
Jeff Johnson (n3npq)
Changed in rpm:
status: Confirmed → Invalid
milestone: 5.4.13 → 5.4.14
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.