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.
