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.