[Spice-devel] [vdagent-win PATCH v6 2/5] Initial rewrite of image conversion code

[Christophe de Dinechin]

> On 20 Jul 2017, at 11:23, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> On Thu, Jul 20, 2017 at 09:42:26AM +0200, Christophe de Dinechin wrote:
>>> I moved to "unsigned int" 2 versions ago.
>> 
>> It was courteous of you.
> 
> Yup, thanks, did not take a look at the newer iterations yet.
> 
>> 
>>> 
>>> But still think that is a useful discussion. But honestly I think
>>> in this case the readability is quite an opinion and for me
>>> unsigned is like long, perfectly readable and I saw lot of code
>>> using just unsigned.
>> 
>> It is useful on two levels:
>> 
>> a) Because I personally feel ‘unsigned int’ less readable and less
>> consistent with the common usage for.’long’ or ‘short’.
> 
> Talking about consistency, should why are you using 'int' rather than
> 'signed' if you prefer using 'unsigned’?

For the same reason that I prefer ‘unsigned’ over ‘unsigned int’ : because
it is the shortest way to express the given type. Remember ‘WET’,
“We Enjoy Typing” ? :-)

> Another way of seeing
> consistency is:
> 
> unsigned long
> unsigned short
> unsigned int

Unsigned long and unsigned short cannot be written in any other way
without a typedef. But many developers have typedefs such as
ulong or ushort to avoid what they see as an inconsistency, having
types that require two words when the vast majority of types require
only one (see below one implication for C++).

> 
> You've got the unsigned modifier, and then something giving a hint at
> the storage size.
> 
> Similarly, you have:
> 
> long
> short
> int
> 
> and you know these usually are signed.

No, they are always signed. Only char is possibly signed or unsigned, AFAIR.
See https://stackoverflow.com/questions/2054939/is-char-signed-or-unsigned-by-default.

> 
>> 
>> b) To discuss the matter of debating style preferences in code reviews,
>> notably for new code and new languages.
>> 
>> In that case, we have C++ code, where my experience tells me that
>> ‘unsigned’ is by far more common (as is camel-case, for example).
>> 
>> If we start considering that some C programmer may not know that
>> ‘unsigned’ means ‘unsigned int’, what happens the first time someone
>> introduces ‘auto’ or ‘for (a : b) in C++ code, or _Generic in C code?
> 
> To reuse your numbers, the usage of unsigned VS unsigned int in arch/ in
> the kernel was 90%/10% or even less than that. So yes, it's not unlikely
> that someone will never have came across "unsigned", or forgot what it
> means, …

Are you saying that since the usage ratio in the Linux kernel is 90%/10%,
it is likely that regular C developers don’t know what ‘unsigned’ means?
I disagree, ‘unsigned’ is not an obscure C feature. A developer who does not
know what ‘unsigned’ means needs to learn it.

> _Generic has no trivial equivalent for C code contrary to
> "unsigned". Your C++ examples being C++ and not C, I would care less
> about C programmers not familiar with C++ ;)

The code under review is C++, isn’t it? C++ users are even more
wary of two-word type names than C users. There are syntactic constructs
in C++ that require a one-word name, notably the new constructor-style
syntax for casts:

% cat /tmp/glop.cpp
int long unsigned typedef ulong;

unsigned ok2 = (unsigned) 1;
unsigned ok1 = unsigned(1);
ulong ok3 = (ulong) 1;
ulong ok4 = ulong(1);
unsigned long ok5 = (unsigned long) 1;
unsigned long bad = unsigned long (1);
ddd at ptitpuce[master] Chapter9> c++ -c /tmp/glop.cpp
/tmp/glop.cpp:8:30: error: expected '(' for function-style cast or type
      construction
unsigned long bad = unsigned long (1);
                    ~~~~~~~~ ^
1 error generated.

This is one of the reasons for this preference for ‘unsigned’ of all
serious C++ developers that I know of (that may be a possible reason
Frediano unconsciously started with ‘unsigned’ in C++ code):


I’m not sure the discussion is very productive. I only answered because
I thought there was a factual error about int being ‘usually’ signed
and because I thought it was worth explaining the C++ preference for
single-word type names. That being said, I am not sure I can convince
you, so we’ll probably agree to disagree on this one ;-)


Cheers,
Christophe

> 
> Christophe
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel