Re: Removing forced casts between signed/unsigned char pointers

[DervishD <zsh@xxxxxxxxxxxx>]

    Hi Wayne :)

 * Wayne Davison <wayned@xxxxxxxxxxxxxxxxxxxxx> dixit:
> I first looked at the easy fix of adding a few new forced casts,
> but I didn't like how many casts to and from "unsigned char *" and
> "char *" we were getting in the code.  Instead, I decided to change
> some of the functions and global pointers that were
> taking/returning unsigned char pointers to make them take/return
> normal char pointers.  This allowed me to remove a bunch of forced
> casts.

    But that's not a good idea. If you go for portability, you
shouldn't assume that a plain char is equivalent to "unsigned char".
If you don't mind me suggesting, you should use "unsigned char"
wherever possible, and cast to plain "char" when needed. In fact,
plain char has been maintained in the C99 standard because making it
dissappear or making it an alias to "unsigned char" will break some
existing code.
 
> I was careful in my changes to preserve any unsigned features of
> the old code (for instance, ztrcmp() still compares the character
> values in an unsigned manner even though it takes normal "char *"
> strings).

    If you really want to make sure you're preserving unsigned
features, avoid the use of "char" and "char *". I know you're using
autoconf and that anyway you can check if "char" is equivalent to
"unsigned char" or "signed char" using SCHAR_MAX, CHAR_MAX and
UCHAR_MAX macros, but I think that using "unsigned char" where
possible is safer.

    I haven't seen any potential problem in your patch, but I don't
think it's a good idea to change some prototypes from using "unsigned
char *" to "char *". I have had a similar problem with one of my
projects and I think that the extra casts are a better solution,
especially if you can afford to modify prototypes. You can run into
trouble by changing from "unsigned char" to "char" (this applies to
pointers to these types, too), because you can potentially restrict
the range, but unless your code relies on "char" being signed at some
point, you can safely go from "char" to "unsigned char".

    Currently I don't know of any system nor C implementation that
won't copy eight bits from any kind of char type to any other kind of
char type (even through pointers) if CHAR_BIT is 8, and the real
danger is probably in comparisons, but anyway, and to be on the safe
side, I would go for "unsigned char" and will do any needed casts.
Given that you can change some prototypes, I think that the number of
casts will be pretty low. In my project, going to "unsigned char" and
"unsigned char *" will need casts only for a couple uses of
<string.h> functions (which should be avoided whenever possible) and
a couple of Xlib functions. The other way around (using "char" &
"char *" instead) will need lots of casts and a careful auditing to
see if we are breaking unsigned features, since the code must be 8bit
clean.

    And thanks a lot for fixing the warnings :) I know that sometimes
GCC warnings are nonsenses or misleading, but this time they're
right: the C99 standard makes pretty clear that, no matter if "char"
is equivalent to "unsigned char" or "signed char", the three types
are distinct, so their pointers are different and non compatible, so
you need explicit casts. You can see a discussion in the GCC
bugzilla, and if you want I can point you to the relevant C99
standard paragraphs.

    Raúl Núñez de Arenas Coronado

-- 
Linux Registered User 88736 | http://www.dervishd.net
http://www.pleyades.net & http://www.gotesdelluna.net
It's my PC and I'll cry if I want to...