[Fedora-directory-commits] ldapserver/lib/ldaputil certmap.c, 1.8, 1.9 ldapauth.c, 1.7, 1.8 ldapdb.c, 1.6, 1.7

[Richard Allen Megginson]

Author: rmeggins

Update of /cvs/dirsec/ldapserver/lib/ldaputil
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv26931/ldapserver/lib/ldaputil

Modified Files:
	certmap.c ldapauth.c ldapdb.c 
Log Message:
Bug Description: Need to address 64-bit compiler warnings - part 1
Reviewed by: nhosoi (Thanks!)
Fix Description: The intptr_t and uintptr_t are types which are defined as integer types that are the same size as the pointer (void *) type.  On the platforms we currently support, this is the same as long and unsigned long, respectively (ILP32 and LP64).  However, intptr_t and uintptr_t are more portable.  These can be used to assign a value passed as a void * to get an integer value, then "cast down" to an int or PRBool, and vice versa.  This seems to be a common idiom in other applications where values must be passed as void *.
For the printf/scanf formats, there is a standard header called inttypes.h which defines formats to use for various 64 bit quantities, so that you don't need to figure out if you have to use %lld or %ld for a 64-bit value - you just use PRId64 which is set to the correct value.  I also assumed that size_t is defined as the same size as a pointer so I used the PRIuPTR format macro for size_t.
I removed many unused variables and some unused functions.
I put parentheses around assignments in conditional expressions to tell the compiler not to complain about them.
I cleaned up some #defines that were defined more than once.
I commented out some unused goto labels.
Some of our header files shared among several source files define static variables.  I made it so that those variables are not defined unless a macro is set in the source file.  This avoids a lot of unused variable warnings.
I added some return values to functions that were declared as returning a value but did not return a value.  In all of these cases no one was checking the return value anyway.
I put explicit parentheses around cases like this: expr || expr && expr - the && has greater precedence than the ||.  The compiler complains because it wants you to make sure you mean expr || (expr && expr), not (expr || expr) && expr.
I cleaned up several places where the compiler was complaining about possible use of uninitialized variables.  There are still a lot of these cases remaining.
There are a lot of warnings like this:
lib/ldaputil/certmap.c:1279: warning: dereferencing type-punned pointer will break strict-aliasing rules
These are due to our use of void ** to pass in addresses of addresses of structures.  Many of these are calls to slapi_ch_free, but many are not - they are cases where we do not know what the type is going to be and may have to cast and modify the structure or pointer.  I started replacing the calls to slapi_ch_free with slapi_ch_free_string, but there are many many more that need to be fixed.
The dblayer code also contains a fix for https://bugzilla.redhat.com/show_bug.cgi?id=463991 - instead of checking for dbenv->foo_handle to see if a db "feature" is enabled, instead check the flags passed to open the dbenv.  This works for bdb 4.2 through bdb 4.7 and probably other releases as well.
Platforms tested: RHEL5 x86_64, Fedora 8 i386
Flag Day: no
Doc impact: no



Index: certmap.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/lib/ldaputil/certmap.c,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- certmap.c	18 Oct 2007 00:08:36 -0000	1.8
+++ certmap.c	8 Oct 2008 17:29:05 -0000	1.9
@@ -55,6 +55,7 @@
 
 #include <key.h>
 #include <cert.h>
+#define DEFINE_LDAPU_STRINGS 1
 #include <ldaputil/certmap.h>
 #include <ldaputil/ldapauth.h>
 #include <ldaputil/errors.h>
@@ -300,7 +301,8 @@
     int rv;
 
     while(node) {
-	rv = (int)(*print_fn)(node->info, pinfo);
+	uintptr_t retval = (uintptr_t)(*print_fn)(node->info, pinfo);
+	rv = (int)retval;
 	if (rv != LDAPU_SUCCESS) return rv;
 	node = node->next;
     }
@@ -1691,6 +1693,7 @@
     char *ptr;
     int eof;
     int rv;
+    uintptr_t retval;
     LDAPUPrintInfo_t pinfo;
     
 #ifdef XP_WIN32
@@ -1730,7 +1733,8 @@
     pinfo.fp = tfp;
     pinfo.arg = default_certmap_info->issuerName;
 
-    rv = (int)ldapu_certinfo_print (default_certmap_info, &pinfo);
+    retval = (uintptr_t)ldapu_certinfo_print (default_certmap_info, &pinfo);
+    rv = (int)retval;
 
     if (rv != LDAPU_SUCCESS) {
 	fclose(tfp);


Index: ldapauth.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/lib/ldaputil/ldapauth.c,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- ldapauth.c	10 Nov 2006 23:46:00 -0000	1.7
+++ ldapauth.c	8 Oct 2008 17:29:05 -0000	1.8
@@ -53,6 +53,7 @@
 #include <ldap.h>
 #include <prprf.h>
 
+#define DEFINE_LDAPU_STRINGS 1
 #include <ldaputil/certmap.h>
 #include <ldaputil/errors.h>
 #include <ldaputil/ldapauth.h>


Index: ldapdb.c
===================================================================
RCS file: /cvs/dirsec/ldapserver/lib/ldaputil/ldapdb.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- ldapdb.c	10 Nov 2006 23:46:00 -0000	1.6
+++ ldapdb.c	8 Oct 2008 17:29:05 -0000	1.7
@@ -171,7 +171,10 @@
 	LDAPHostEnt *result, char *buffer, int buflen, int *statusp,
 	void *extradata )
 {
+    /* old code did this which was clearly wrong:
     return( (LDAPHostEnt *)PR_GetError() );
+    which leads me to believe this is not used */
+    return( NULL );
 }
 #endif /* LDAP_OPT_DNS_FN_PTRS */