For example in src/lib/file-cache.c: if (cache->mmap_base == MAP_FAILED) { Should be fixed, probably like this: if ((int)cache->mmap_base == MAP_FAILED) { There are a lot of occurences for these in the source. GCC only warns because of this, but DEC C is known to consider this an error: Error: file-cache.c, line 79: In this statement, "new_base" and "(-1)" may not be compared for equality or inequality. if (new_base == MAP_FAILED) { And indeed, unsigned integers and -1 are rarely equal. -- Gabucino
On 30.8.2007, at 15.49, B?rczi G?bor (Gabucino) wrote:> For example in src/lib/file-cache.c: > > if (cache->mmap_base == MAP_FAILED) { > > Should be fixed, probably like this: > > if ((int)cache->mmap_base == MAP_FAILED) {No. MAP_FAILED is supposed to be a pointer. For example in Linux: /usr/include/sys/mman.h:#define MAP_FAILED ((void *) -1) And in any case you really don't want to cast it to int, rather ssize_t so that it won't break with 64bit systems.> And indeed, unsigned integers and -1 are rarely equal.Not really, Dovecot does such comparisons a lot. :) Casting an unsigned integer to signed where the original value doesn't fit to the destination is undefined by ANSI C. Casting a signed integer to unsigned integer however is valid. (unsigned int)-1 means the largest value that the unsigned int can hold. So comparing unsigned int to -1 means comparing it to the largest value. -------------- next part -------------- A non-text attachment was scrubbed... Name: PGP.sig Type: application/pgp-signature Size: 186 bytes Desc: This is a digitally signed message part URL: <http://dovecot.org/pipermail/dovecot/attachments/20070830/1afa3002/attachment-0002.bin>
On Digital Unix V4.0B, it is: /usr/include/sys/mman.h:#define MAP_FAILED (-1L) /* unsuccessful return from mmap() */ On Aug 30, 2007, at 6:10 PM, Timo Sirainen wrote:> On 30.8.2007, at 15.49, B?rczi G?bor (Gabucino) wrote: > >> For example in src/lib/file-cache.c: >> >> if (cache->mmap_base == MAP_FAILED) { >> >> Should be fixed, probably like this: >> >> if ((int)cache->mmap_base == MAP_FAILED) { > > No. MAP_FAILED is supposed to be a pointer. For example in Linux: > > /usr/include/sys/mman.h:#define MAP_FAILED ((void *) -1) > > And in any case you really don't want to cast it to int, rather > ssize_t so that it won't break with 64bit systems. > >> And indeed, unsigned integers and -1 are rarely equal. > > Not really, Dovecot does such comparisons a lot. :) > > Casting an unsigned integer to signed where the original value > doesn't fit to the destination is undefined by ANSI C. Casting a > signed integer to unsigned integer however is valid. (unsigned > int)-1 means the largest value that the unsigned int can hold. So > comparing unsigned int to -1 means comparing it to the largest value. >