On Tue, Sep 03, 2013 at 10:29:57AM +0200, Lukas Slebodnik wrote: > On (02/09/13 22:55), Jakub Hrozek wrote: > >On Mon, Sep 02, 2013 at 03:20:12PM -0400, Simo Sorce wrote: > >> On Mon, 2013-09-02 at 19:18 +0200, Lukas Slebodnik wrote: > >> > ehlo, > >> > > >> > Some platforms can have defined SIZE_T_MAX. > >> > It is better to use conditional build. > >> > > >> > Two patches are attached. one for master(1.10) and second for 1.9 > >> > >> > >> Uhmm defining SIZE_T_MAX as (size_t)-1 is not technically correct, it > >> may work with gcc, but could fail with an optimizer, as -1 is simply an > >> illegal value for an unsigned quantity. > >> > >> We should use the actual maximum value here. > >> > >> Simo. > > > >tl;dr - I think we should simply use SIZE_MAX instead. > > > >I actually think Lukas' patch *improves* the code. It seems that the > >non-patched code defined SIZE_T_MAX no matter what. Lukas simply wraps > >the macro #define in an #ifdef so if a platform defined SIZE_T_MAX in > >some kind of system wide header, the definition would be picked up. > > > >But the real question I see is why did we ever use SIZE_T_MAX where we > >should have used SIZE_MAX ? The standard says (7.18.3) that size_t > >maximum value is SIZE_MAX with the minimum value set to 65535 where > >implementations can choose larger values. > > > >Also, what problem do you see with (size_t)-1 ? As far as I see, this > >is the recommended approach. The only alternative I can think of is ~0 > >but in several discussions (on comp.lang.c.moderated) this was considered > >unsafe on some machines. > > I agree with SIZE_MAX. > Here is part of header file stdint.h > /* Limit of `size_t' type. */ > # if __WORDSIZE == 64 > # define SIZE_MAX (18446744073709551615UL) > # else > # define SIZE_MAX (4294967295U) > # endif > > New patches attached and subject is also changed. > > LS
Yep, that aligns with some testing I did yesterday. Since overflows are still being tested with unit tests, I think the patch is fine. ACK. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
