compile time warnings (pointer to int casts) on 64bit architectures

Sven Mueller cyrus at incase.de
Thu Jun 8 12:23:31 EDT 2006


Hi.

I'm one of the maintainers of the cyrus-imapd-2.2 packages in Debian. We
received a report (one might call it bug report or simply a wishlist
entry) about compile time warnings on 64 bit architectures for our
2.2.13 package. While I know that cyrus-imapd-2.2.13 works quite nicely
on AMD64, the warnings are still a bit unfortunate to encounter. I
therefore tried today and created patches for the three files affected
by this particular warning (cast from pointer to integer of different
size). Please find it attached.

The first part, affecting imap/squat_build.c, replaces test against the
least significant bit of a pointer (in an integer realm) with a test
against the least significant bit of an integer from the same union,
which should the very same bit in memory. This change is done at three
different places in squat_build.c (first tests for the bit to be 0 where
the pointer is non-NULL, the other two test for the bit to be non-0).

The second part, affecting imap/tls.c, fixes the output of a log message
to use size_t as the size specifier instead of (unsigned) int. The
format string for printf is changed accordingly ("z" uses a size_t
argument, and sizeof(size_t) is guaranteed to be sizeof(void*)). An
alternate format could have been "%p" instead of "%016zX" which would
work with any size of void* instead of needing to be adjusted to the
actual size of the void* argument. But it prints a "0x" respectively
"0X" in front of the actual hex number (this is not a disadvantage IMHO).

The third part, affecting master/master.c, removes the ignore_err local
variable and replaces it with what it actually should do (but is not
guaranteed to do on non-32bit architectures): Test wether the "rock"
pararmeter is non-NULL. An alternative would have been to replace
    int ignore_err = (int) rock;
with
    int ignore_err = (int) (rock!=NULL);
which should be logically equivalent to the patch I made.

Anyway, feedback and corrections are welcome (as would be the
integration into a future release of cyrus-imapd).

Regards,
Sven
-------------- next part --------------
diff -urNad --exclude=CVS --exclude=.svn ./imap/squat_build.c /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/imap/squat_build.c
--- ./imap/squat_build.c	2006-04-25 17:28:58.000000000 +0200
+++ /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/imap/squat_build.c	2006-06-08 14:33:06.966146056 +0200
@@ -479,7 +479,7 @@
     for (i = 0; i < VECTOR_SIZE(t->entries); i++) {
       SquatWordTableEntry* e = &(t->entries[i]);
       
-      if (e->leaf_presence != NULL && ((int)e->leaf_presence & 1) == 0) {
+      if (e->leaf_presence != NULL && ((int)e->leaf_presence_singleton & 1) == 0) {
         free(e->leaf_presence);
       }
     }
@@ -568,7 +568,7 @@
 
   if (word_entry == NULL) {
     /* We are in "per document" mode. */
-    if (((int)e->leaf_presence & 1) != 0) {
+    if (((int)e->leaf_presence_singleton & 1) != 0) {
       /* We currently have a singleton here. */
       int oldch = e->leaf_presence_singleton >> 1;
 
@@ -787,7 +787,7 @@
 
       word[0] = (char)i;
 
-      if (((int)e->leaf_presence & 1) != 0) {
+      if (((int)e->leaf_presence_singleton & 1) != 0) {
 	/* Got a singleton at this branch point. Just output the single word. */
         word[1] = (char)(e->leaf_presence_singleton >> 1);
         e->leaf_presence = NULL; /* clear the leaf out */
diff -urNad --exclude=CVS --exclude=.svn ./imap/tls.c /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/imap/tls.c
--- ./imap/tls.c	2006-04-25 17:28:58.000000000 +0200
+++ /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/imap/tls.c	2006-06-08 14:28:13.259629963 +0200
@@ -717,14 +717,22 @@
 	return (ret);
 
     if (cmd == (BIO_CB_READ | BIO_CB_RETURN)) {
-	printf("read from %08X [%08lX] (%d bytes => %ld (0x%X))",
-	       (unsigned int) bio, (long unsigned int) argp,
+#if __WORDSIZE == 64
+	printf("read from %016zX [%016zX] (%d bytes => %ld (0x%X))",
+#else
+	printf("read from %08zX [%08zX] (%d bytes => %ld (0x%X))",
+#endif
+	       (size_t) bio, (size_t) argp,
 	       argi, ret, (unsigned int) ret);
 	tls_dump(argp, (int) ret);
 	return (ret);
     } else if (cmd == (BIO_CB_WRITE | BIO_CB_RETURN)) {
-	printf("write to %08X [%08lX] (%d bytes => %ld (0x%X))",
-	       (unsigned int) bio, (long unsigned int)argp,
+#if __WORDSIZE == 64
+	printf("write to %016zX [%016zX] (%d bytes => %ld (0x%X))",
+#else
+	printf("write to %08zX [%08zX] (%d bytes => %ld (0x%X))",
+#endif
+	       (size_t) bio, (size_t)argp,
 	       argi, ret, (unsigned int) ret);
 	tls_dump(argp, (int) ret);
     }
diff -urNad --exclude=CVS --exclude=.svn ./master/master.c /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/master/master.c
--- ./master/master.c	2006-04-25 17:28:58.000000000 +0200
+++ /tmp/dpep-work.MJclMQ/cyrus-imapd-2.2.13/master/master.c	2006-06-08 14:28:13.259629963 +0200
@@ -1304,7 +1304,6 @@
 
 void add_service(const char *name, struct entry *e, void *rock)
 {
-    int ignore_err = (int) rock;
     char *cmd = xstrdup(masterconf_getstring(e, "cmd", ""));
     int prefork = masterconf_getint(e, "prefork", 0);
     int babysit = masterconf_getswitch(e, "babysit", 0);
@@ -1324,7 +1323,7 @@
 	snprintf(buf, sizeof(buf),
 		 "unable to find command or port for service '%s'", name);
 
-	if (ignore_err) {
+	if (rock != NULL) {
 	    syslog(LOG_WARNING, "WARNING: %s -- ignored", buf);
 	    return;
 	}
@@ -1340,7 +1339,7 @@
 	/* must have empty/same service name, listen and proto */
 	if ((!Services[i].name || !strcmp(Services[i].name, name)) &&
 	    (!Services[i].listen || !strcmp(Services[i].listen, listen)) &&
-	    (!Services[i].proto || !strcmp(Services[i].proto, proto)))
+	    ((Services[i].proto==NULL) || !strcmp(Services[i].proto, proto)))
 	    break;
     }
 
@@ -1349,7 +1348,7 @@
 	char buf[256];
 	snprintf(buf, sizeof(buf), "multiple entries for service '%s'", name);
 
-	if (ignore_err) {
+	if (rock != NULL) {
 	    syslog(LOG_WARNING, "WARNING: %s -- ignored", buf);
 	    return;
 	}
@@ -1438,7 +1437,6 @@
 
 void add_event(const char *name, struct entry *e, void *rock)
 {
-    int ignore_err = (int) rock;
     char *cmd = xstrdup(masterconf_getstring(e, "cmd", ""));
     int period = 60 * masterconf_getint(e, "period", 0);
     int at = masterconf_getint(e, "at", -1), hour, min;
@@ -1450,7 +1448,7 @@
 	snprintf(buf, sizeof(buf),
 		 "unable to find command or port for event '%s'", name);
 
-	if (ignore_err) {
+	if (rock != NULL) {
 	    syslog(LOG_WARNING, "WARNING: %s -- ignored", buf);
 	    return;
 	}


More information about the Cyrus-devel mailing list