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 <sgall...@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.
--
Simo Sorce * Red Hat, Inc * New York
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
