[SSSD] [PATCH] Properly detect negative/invalid values for the minId and maxId
Stephen Gallagher
sgallagh at redhat.com
Thu Sep 10 20:57:06 UTC 2009
New patch with code review concerns addressed.
On 09/10/2009 04:37 PM, Simo Sorce wrote:
> On Thu, 2009-09-10 at 16:02 -0400, Stephen Gallagher wrote:
>> New approach attached.
>>
>> Patch 0001: Create two convenience functions, strtouint32 and
>> strtoint32. These behave identically to strtol or strtoll, except that
>> they are guaranteed to be constrained to 32 bits.
>
> I don't like the ifdefs, we should always use long long and bail out in
> configure if it is not a 64bit quantity.
>
> Other comments inline.
>
>> Patch 0002: Read the minId and maxId values as a string, testing them
>> for negation and zero-length before passing them to the function
>> created
>> in patch 0001 to convert it to a 32-bit value.
>
> Ack this one.
>
>
>
>>
>>
>>
>>
>>
>> plain text
>> document
>> attachment
>> (0001-Add-strtoint32-and-strtouint32-convenience-functions.patch)
>
> [..]
>
>> diff --git a/server/util/strtonum.c b/server/util/strtonum.c
>> new file mode 100644
>> index 0000000..f7fa5a5
>> --- /dev/null
>> +++ b/server/util/strtonum.c
>> @@ -0,0 +1,120 @@
>> +/*
>> + SSSD
>> +
>> + InfoPipe
>
> errr ^^^^^^^^ ?
>
>> + Copyright (C) Stephen Gallagher <sgallagh at redhat.com> 2009
>
> [..]
>
>> +/* strtoint32 */
>> +uint32_t strtoint32(const char *nptr, char **endptr, int base)
>
> ^^^^^^^^ I guess this should be int32_t not unit32_t ...
>
>> +{
>> +#if SIZEOF_LONG>=4
>> + long ret = 0;
>> + errno = 0;
>> + ret = strtoul(nptr, endptr, base);
>
> and this should be strtol(), not strtoul() ... these defines are bad for
> this kind of errors exactly, please avoid it and always use long long.
>
> [..]
>
>> diff --git a/server/util/strtonum.h b/server/util/strtonum.h
>> new file mode 100644
>> index 0000000..ba4cf16
>> --- /dev/null
>> +++ b/server/util/strtonum.h
>> @@ -0,0 +1,32 @@
>
> [..]
>
>> +#ifndef _STRTONUM_H_
>> +#define _STRTONUM_H_
>> +
>> +#include <ctype.h>
>> +#include <stdlib.h>
>> +#include <stdint.h>
>> +
>> +uint32_t strtoint32(const char *nptr, char **endptr, int base);
>
> ^^^^^^^^ again int32_t not uint32_t
>
>> +uint32_t strtouint32(const char *nptr, char **endptr, int base);
>> +
>> +#endif /* _STRTONUM_H_ */
>
>
> The rest looks ok.
>
> Simo.
>
--
Stephen Gallagher
RHCE 804006346421761
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-Add-strtoint32-and-strtouint32-convenience-functions.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20090910/d7604a08/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0002-Properly-detect-negative-invalid-values-for-the-minI.patch
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20090910/d7604a08/attachment-0001.ksh>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20090910/d7604a08/attachment.sig>
More information about the sssd-devel
mailing list