On Sunday 11 October 2009 12:01, Cristian Ionescu-Idbohrn wrote:
> I'd like to point you to this comment in Makefile.flags:
>
> # FIXME: These warnings are at least partially to be concerned about and
> # should be fixed..
> #CFLAGS += $(call cc-option,-Wconversion,)
>
> I just removed the '#' in front of CFLAGS and found myself staring at
> some 131 conversion warnings in shell/ash.c only:
>
> 27 'size_t' from 'int' may change the sign of the result
> 24 'char' from 'int' may alter its value
> 15 'smallint' from 'int' may alter its value
> 15 'unsigned int' from 'int' may change the sign of the result
> 13 'uint8_t' from 'int' may alter its value
> 10 'int' from 'unsigned int' may change the sign of the result
> 8 'int' from 'size_t' may change the sign of the result
> 3 'int32_t' from 'uint32_t' may change the sign of the result
> 2 'size_t' from 'ssize_t' may change the sign of the result
> 2 'ssize_t' from 'size_t' may change the sign of the result
> 2 'uint32_t' from 'int' may change the sign of the result
> 1 'cc_t' from 'int' may alter its value
> 1 'long long unsigned int' from 'int' may change the sign of the result
> 1 'mode_t' from 'int' may change the sign of the result
> 1 'rlim_t' from 'long int' may change the sign of the result
> 1 'token_id_t' from 'char' may change the sign of the result
> 1 'token_id_t' from 'int' may alter its value
> 1 'uint32_t' from 'int32_t' may change the sign of the result
> 1 'uint32_t' from 'long long unsigned int' may alter its value
> 1 'unsigned char' from 'int' may alter its value
> 1 'unsigned int' from 'pid_t' may change the sign of the result
And the total is staggering:
# grep warning make.log | wc -l
3153
Seems like significant part of them are false positives.
for example:
for (i = 0; i < nblock; i++) {
j = eclass8[i];
k = ftab[j] - 1;
ftab[j] = k;
fmap[k] = i; <-------
}
archival/bz/blocksort.c:260: warning: conversion to 'uint32_t' from 'int32_t'
may change the sign of the result
What to do? fmap[k] = (uint32_t)i; ?
Or this:
k = fmap[i] - H;
if (k < 0)
k += nblock;
eclass[k] = j;
fmap[i] and H are uint32_t, k is int32_t, and it CAN be negative.
> I thought it might be a good idea to initiate a discussion around the
> subject, and try to find a way to correct this. Maybe that was a not
> such a good idea.
If you can deal with some if them by changing signedness of the variables,
that is ok.
Adding casts, and widening variables is less desirable.
> On Sun, 11 Oct 2009, Denys Vlasenko wrote:
> >
> > Cristian, size_t is 64-bit on x86-64.
>
> Well, you are probably refering to modifications like this one:
>
> - int i = strlen(str->text);
> + size_t i = strlen(str->text);
>
> But 'i' must obviously be some unsigned type, shouldn't it?
> What's the gain of using 'int' vs. 'size_t'?
int is possibly smaller than size_t.
> > Using size_t for strlen and such bloats code.
>
> size_t _should be used_ for strlen. It's documented. There's a man
> page:
>
> SYNOPSIS
> #include <string.h>
> size_t strlen(const char *s);
>
> It has been there since 1993. And it didn't change since, AFAIK.
Prototype's return type is for compiler to know in which register(s)
it can find the result (i.e. how wide is it).
There is no hard rule "you must not use implicit narrowing
conversion, ever".
> > Do you really think strings longer than 2 gigabytes
> > are typical enough to worry about?
>
> I'm no longer that young and worry free like you are :)
> CMIIW, but robustness is not a negative quality.
How do you survive the code which uses xmalloc
or xasprintf then? It can fail, you know...
"sky is falling, run for your lives!" ?...
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
