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
LS
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.
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.
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
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.
On Tue, Sep 03, 2013 at 02:12:01PM +0200, Jakub Hrozek wrote:
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.
Pushed to master, sssd-1-10 and sssd-1-9
On Tue, 2013-09-03 at 10:29 +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.
If the preprocessor could handle sizeof() it would be neat to do #if sizeof(size_t) == 8 ...
Simo.
On Mon, Sep 02, 2013 at 10:55:55PM +0200, 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.
I think using SIZE_MAX is a good idea here since the standard defines it as the limit of size_t. I introduced SIZE_T_MAX to check for overflows on variables of type size_t. So even if the implementation does not choose to change 65535 to the actual maximal value of size_t the checks will still work, but will limit to corresponding operations to 16bits although more might be possible.
bye, Sumit
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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Simo Sorce -
Sumit Bose