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

[Frediano Ziglio]

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

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

So let's write "long int" for anything. "unsigned" is not less typing,
it's a type specified by the language.

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

I personally have seen tons of "unsigned" reading code.
If some C coders don't know that syntax they can learn it.

Frediano