[Dovecot] array code issue ?

[Timo Sirainen]

On 20.4.2012, at 17.27, Tim Ruehsen wrote:

> I just took a look into the dovecot 2.1 sources and just saw a possible issue 
> in array.h.
> 
> This code snippet as an example:
> #static inline void *
> #array_get_modifiable_i(struct array *array, unsigned int *count_r)
> #{
> #	*count_r = array->buffer->used / array->element_size;
> #	return buffer_get_modifiable_data(array->buffer, NULL);
> #}
> 
> array->buffer->used and array->element_size are of type 'size_t' which is 
> 64bit on amd64 and others while 'count_r' is a 32bit value. At least, I see 
> ugly warnings with -Wconversion (which I personally like to use).

I've been planning on trying out some of clang's warning flags. Last time I used -Wconversion with gcc it was giving way too many warnings to be usable, but clang's -Wconversion looked better when I quickly looked at it.

> I know, it is unlikely that 'array->buffer->used / array->element_size' 
> exceeds 32bit range. But then, dovecot's source is so well written, that the 
> above code seems to disturb dovecot's code aesthetics.

:) Yeah, I intentionally decided to use unsigned int here. It's a bit of wasteful and ugly to use size_t everywhere.. I guess the code could be made something like:

size_t count = array->buffer->used / array->element_size;
I_assert(count < UINT_MAX);
*count_r = (unsigned int)count;

Or something like that. Although these array functions are sometimes in performance critical paths, so adding extra code isn't very good either. Perhaps a simple cast to make the warning go away.. Probably the element_size could also be changed to be unsigned int.

> And who knows... in a few years (when we have THz and TBytes on our desktops) 
> emails (and array sizes) might exceed everything that we think of today.

The email sizes yes, but probably not the number of emails in a mailbox.