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

[Christophe de Dinechin]

> On 19 Jul 2017, at 19:21, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
>> 
>> On Wed, Jul 19, 2017 at 08:03:49AM -0400, Frediano Ziglio wrote:
>>>> 
>>>> On Wed, Jul 19, 2017 at 12:09:23PM +0200, Christophe de Dinechin wrote:
>>>>> 
>>>>>> On 19 Jul 2017, at 11:21, Christophe Fergeau <cfergeau at redhat.com>
>>>>>> wrote:
>>>>>> 
>>>>>> On Wed, Jul 19, 2017 at 10:23:30AM +0200, Christophe de Dinechin
>>>>>> wrote:
>>>>>>> 
>>>>>>>> On 18 Jul 2017, at 17:28, Christophe Fergeau <cfergeau at redhat.com>
>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>> On Mon, Jul 17, 2017 at 11:01:22AM +0100, Frediano Ziglio wrote:
>>>>>>>>> Remove CxImage linking.
>>>>>>>>> Support Windows BMP format.
>>>>>>>> 
>>>>>>>> Too bad there is no small/maintained library which would do that
>>>>>>>> for us
>>>>>>>> :-/ From a quick glance, looks ok.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> +static inline size_t compute_dib_stride(unsigned width, unsigned
>>>>>>>>> bit_count)
>>>>>>>> 
>>>>>>>> Can you use full type names, unsigned int?
>>>>>>> 
>>>>>>> No. Really, no ;-) Otherwise, for consistency, you should replace
>>>>>>> ‘int’
>>>>>>> with ‘signed int’,
>>>>>> 
>>>>>> The way I see it, 'signed'/'unsigned' are type modifiers, 'int' is an
>>>>>> actual type name.
>>>>> 
>>>>> Yes. But ‘long’ is not. It is also a modifier. So why allow “long” or
>>>>> “short" but not “unsigned”?
>>>>> Or are you also writing “long int” and “short int”?
>>>> 
>>>> long/short are enough to make the storage size of the integer obvious,
>>>> even if you don't know that long means long int.
>>>> "unsigned" does not make this obvious unless you know that "unsigned"
>>>> means "unsigned int"
>>>> 
>>> 
>>> Section 6.7.2 of C99 standard specified "unsigned" as type.
>>> The fact you are not familiar with this is an opinion I don't
>>> personally share. "long" does not specify a type as "unsigned"
>>> doesn't.
>>> 
>> 
>> [...]
>> 
>>> 
>>> So let's write "long int" for anything. "unsigned" is not less typing,
>>> it's a type specified by the language.
>> 
>> I never said "unsigned" is not standard compliant, so I don't know why
>> you keep coming back to that.
>> I previously said that just because something is standard-compliant does
>> not mean it's a good idea to do it, [insert your favourite obfuscated C
>> contest example here].
>> 
>> In this particular case, since you feel strongly about it, feel free to
>> ignore my comment, but I'll nonetheless keep thinking it makes things
>> less readable ;)
>> 
>> Christophe
>> 
> 
> I moved to "unsigned int" 2 versions ago.

It was courteous of you.

> 
> 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’.

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?

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org <mailto:Spice-devel at lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/spice-devel <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170720/efc63e16/attachment-0001.html>