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

[Christophe Fergeau]

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"

> 
> > Huge difference to me.
> 
> No, really not, at least as far as C and usage are concerned. It’s
> just a personal preference.  So if Frediano prefers to write
> ‘unsigned’, I think it’s OK, and I will most likely write the same
> way.

I'll byte here, in general (not necessarily in this case) "it's just
personal preference" is not a good argument at all, we want a consistent
codebase, ideally one which is readable. Just because something is valid with
the C standard does not mean saying "personal preference" is a good
justification for using it (and still generally speaking, less typing
often means something less readable, and you read code more than you
write it).

> 
> 
> > you cannot guess the range of the values you
> > can store in there. So let's just be specific.
> 
> The language is quite clear that ‘unsigned’ means ‘unsigned int’.
> There is no guessing involved whatsoever. The guessing about
> the size is because C is not specific about the size of ‘int’ in bits,
> which is why we have <stdint.h>.

Yes, the spec is clear about it. The proportion of C coders who will
know when reading "unsigned" that it means "unsigned int" is what I'm
questioning here, and why I'm pushing for a more specific type.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170719/be49bfea/attachment.sig>