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 000..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 000..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/
From 018a3b69840f01d5e94dedcd8f680253bcc0f546 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher sgall...@redhat.com
Date: Thu, 10 Sep 2009 15:56:52 -0400
Subject: [PATCH 1/4] Add strtoint32 and strtouint32 convenience functions
---
server/Makefile.am |2 +
server/configure.ac |1 +
server/external/sizes.m4 | 44 ++
server/util/strtonum.c | 77 ++
server/util/strtonum.h | 32 +++
5 files changed, 156 insertions(+), 0 deletions(-)
create mode 100644 server/external/sizes.m4
create mode 100644 server/util/strtonum.c
create mode 100644 server/util/strtonum.h
diff --git a/server/Makefile.am b/server/Makefile.am
index 27ac01d..18fb699 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -160,6 +160,7 @@ SSSD_UTIL_OBJ = \
util/signal.c \
util/usertools.c \
util/backup_file.c \
+util/strtonum.c \
$(SSSD_DEBUG_OBJ)
SSSD_RESPONDER_OBJ = \
@@ -203,6 +204,7 @@ dist_noinst_HEADERS = \
util/dlinklist.h \
util/sssd-i18n.h \
util/util.h \
+util/strtonum.h \
config.h \
monitor/monitor.h \
monitor/monitor_interfaces.h \
diff --git a/server/configure.ac b/server/configure.ac
index 8e7ea0b..3320507 100644
--- a/server/configure.ac
+++ b/server/configure.ac
@@ -61,6 +61,7 @@ m4_include([external/libpcre.m4])
m4_include([external/krb5.m4])
m4_include([external/libcares.m4])
m4_include([external/docbook.m4])
+m4_include([external/sizes.m4])
m4_include([util/signal.m4])
PKG_CHECK_MODULES([DBUS],[dbus-1])
diff --git a/server/external/sizes.m4 b/server/external/sizes.m4
new file mode 100644
index 000..53df61d
--- /dev/null
+++ b/server/external/sizes.m4
@@ -0,0 +1,44 @@
+# Solaris needs HAVE_LONG_LONG defined
+AC_CHECK_TYPES(long long)
+
+AC_CHECK_SIZEOF(int)
+AC_CHECK_SIZEOF(char)
+AC_CHECK_SIZEOF(short)
+AC_CHECK_SIZEOF(long)
+AC_CHECK_SIZEOF(long long)
+
+if test $ac_cv_sizeof_long_long -lt 8 ; then
+AC_MSG_ERROR([SSSD requires long long of 64-bits])
+fi
+
+AC_CHECK_TYPE(uint_t, unsigned int)
+AC_CHECK_TYPE(int8_t, char)
+AC_CHECK_TYPE(uint8_t, unsigned char)
+AC_CHECK_TYPE(int16_t, short)
+AC_CHECK_TYPE(uint16_t, unsigned short)
+
+if test $ac_cv_sizeof_int -eq 4 ; then
+AC_CHECK_TYPE(int32_t, int)
+AC_CHECK_TYPE(uint32_t, unsigned int)
+elif test $ac_cv_size_long -eq 4 ; then
+AC_CHECK_TYPE(int32_t, long)
+AC_CHECK_TYPE(uint32_t, unsigned long)
+else
+AC_MSG_ERROR([LIBREPLACE no 32-bit type found])
+fi
+
+AC_CHECK_TYPE(int64_t, long long)
+AC_CHECK_TYPE(uint64_t, unsigned long long)
+
+AC_CHECK_TYPE(size_t, unsigned int)
+AC_CHECK_TYPE(ssize_t, int)
+
+AC_CHECK_SIZEOF(off_t)
+AC_CHECK_SIZEOF(size_t)
+AC_CHECK_SIZEOF(ssize_t)
+
+AC_CHECK_TYPE(intptr_t, long long)
+AC_CHECK_TYPE(uintptr_t,
