Re: [PATCH] sys/types.h: Avoid bit-manipulation of signed values

[Corinna Vinschen <vinschen at redhat dot com>]

On Nov 13 09:42, Nick Withers wrote:
> On Thu, 2015-11-12 at 13:57 +0100, Corinna Vinschen wrote:
> > On Nov 12 15:14, Nick Withers wrote:
> > > Sorry, let's try that again...
> > > 
> > > On Thu, 2015-11-12 at 15:11 +1100, Nick Withers wrote:
> > > > Hi all,
> > > > 
> > > > The patch below attempts to resolve potential undefined behaviour
> > > > in
> > > > bit-manipulation in the FD_* macros, also clearing up the GCC
> > > > warning
> > > > "conversion to 'unsigned int' from 'int' may change the sign of
> > > > the
> > > > result" with -Wsign-conversion and an int FD parameter.
> > 
> > Sorry if I'm dense, but... all these macros (NBBY, FD_SETSIZE,
> > howmany)
> > are defined using int values, not unsigned int values, in glibc and
> > everywhere else, e.g. X11/Xpoll.h, python2.7/pyport.h, etc.  I don't
> > think we should use different definitions which potentially collide.
> 
> Righto, I'll submit a new version which does not modify these elements.
> 
> Can I kill howmany() entirely within sys/types.h or is it intended to
> be exposed to userland after the header's included?

It's intended to be exposed from sys/param.h so it could be moved
there and be killed in sys/types.h.

> Should I provide a patch to the linux / winsup FD_ code?

Linux yes, sure.   Winsup, I'm not clear what you mean there.

Apart from this int vs. unsigned int problem:

It's not clear to me why the FD_ macros were placed in sys/types.h at
all.  Historical reasons probably, but it's kind of puzzeling still.
This stuff is used in Cygwin but not anywhere in Newlib.

- NBBY belongs to sys/param.h

- FD_SETSIZE belongs to sys/select.h (Cygwin only)

- NFDBITS -> sys/select.h

- howmany -> sys/param.h

= FD_xxx -> sys/select.h

RTEMS guys, any input on how you handle this stuff?  Do we have to stick
to sys/types.h or shall we carefully rearrange the definitions to be
better aligned with BSD, Linux, et al?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgprjS8bAp2EL.pgp
Description: PGP signature