[Nut-upsdev] gcc4 compiler warnings
Arjen de Korte
nut+devel at de-korte.org
Fri Feb 10 13:10:57 UTC 2006
> On 2/10/06, Arjen de Korte <nut+devel at de-korte.org> wrote:
>>
>> > On Thu, 9 Feb 2006, Charles Lepple wrote:
>> >
>> >> On 2/9/06, Arjen de Korte <nut+devel at de-korte.org> wrote:
>> >>> This is due to the fact that some drivers need to pass character
>> values
>> >>> between 0x80 and 0xFF to or from the drivers, but the serial
>> functions
>> >>> use
>> >>> signed chars. Unless some drivers use negative values, my preference
>> >>> would
>> >>> be to fix/create serial functions (to use unsigned chars), rather
>> than
>> >>> to
>> >>> convert the characters between 0x80 and 0xFF to negative numbers.
>> >>> Especially where status bits are used, listing a hex value is more
>> >>> readable then.
>> >>
>> >> I tend to agree with Arjen on this one - most drivers will be
>> treating
>> >> characters as unsigned, since the majority of the values being
>> >> returned are unsigned.
>> >
>> > Indeed. It didn't even occur to me that the serial functions might use
>> > signed char for input/output. I'm all for fixing this the proposed
>> > (==right) way.
>>
>> In that case, how are we going to implement this? Modify the existing
>> drivers to use unsigned char for all communication with the UPS or
>> create
>> new ones? Be aware that some drivers use string operations on the
>> results
>> received from the UPS (strlen or strcmp for instance) and will require
>> typecasting there (char *). Otherwise we're in for a lot more compiler
>> warnings from string operations.
>
> OK, now I'm starting to see the value of separate functions for
> unsigned char results.. (It would probably be best if they were
> inline...)
I'd prefer switching to unsigned char in the long run and (explicit)
typecasting to signed char where needed/appropriate/not harmful. In case
you're using string operations, this will be required. Probably defining
new functions which use unsigned char and using inline macro's to mimic
the old ones is doable, but I'd rather see that all drivers are converted
to use unsigned chars and only typecast to char where string operations
are involved (and where it is guaranteed not to cause problems).
> What worries me is that in the push to reduce the number of warnings,
> drivers are converted without testing to see if they are actually
> affected by the change.
This worries me too. Personally, I don't like the "sledgehammer" approach
of typecasting unsigned char from the drivers to signed char for the
serial functions at all. I've been bitten by this before and have learned
my lesson then. Typecasting may be useful in some cases, but going from
unsigned to signed (and in many cases, vice versa) is not one of them
IMHO.
>> Nevertheless, I would prefer typecasting (unsigned char *) to (char *)
>> over the reverse, since string operations are usually performed on (7
>> bit)
>> ASCII text only, where there is no real difference between using
>> unsigned
>> and signed chars.
>
> Speaking of which, are there any cases where UTF-8 would cause
> problems? In most cases where string comparisons are being used, the
> UPS should be sending 7-bit values, so this may not be an issue.
Indeed, it probably wouldn't be much an issue. String operations have no
notion of UTF-8 and are only guaranteed to work on ASCII characters
(0x00-0x7F) anyway. So if your UPS speaks UTF-8, you'd better leave the
string functions out, or you may be in for some real unpleasant surprizes
sooner or later.
Regards, Arjen
More information about the Nut-upsdev
mailing list