[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