[Spice-devel] [PATCH spice-gtk v4 12/29] fixup! usb-redir: add files for SCSI and USB MSC implementation

[Frediano Ziglio]

> 
> Frediano Ziglio writes:
> 
> >>
> >> > Do not use G_GUINT32_FORMAT.
> >> > We support a minimum of 32 bit architectures.
> >>
> >> What was wrong with the old code?
> >>
> >
> > Shorter and easier to read. I mean '%u' and '%" G_GUINT32_FORMAT "',
> > if you have an issue is harder to get it.
> > For instance it was hard to spot the misleading '0x%" G_GUINT32_FORMAT "'
> > which got '0x%u' so 100 would be formatted as 0x100 which I would read
> > as 256, not 100.
> 
> Ah, I can understand how this could have been hard to find.
> 
> Still, even if I now understand where you come from, and even if I know
> that your patch does not really hurt in practice given what is currently
> known of the support platforms, I still see that patch as going somewhat
> in the wrong direction as far as portability best practices are concerned.
> 

Do you think we are even going to support 16 bit platforms? This would
require quite a rewrite of the entire project and also of the dependent
libraries. And I don't see a 16 bit program supporting such a large
memory requirements so easily.

> Reviewed-but-not-entirely-approved-by: Christophe de Dinechin
> <dinechin at redhat.com>
> 
> >
> >> If you want to get rid of G_ macro dependencies, why not use PRIu32?
> >>
> >
> > No, G_ macros are fine, also taking into account that PRIu64 and
> > G_GUINT64_FORMAT
> > could be different (for instance in Windows) and that the receiving
> > formatting
> > function accepts better the G_ version (they are GLib).
> 
> This comment, while it reinforces your rationale, made me extremely sad
> ;-)
> 

I did some minor checks yesterday about formatting strings and Windows.
That's a weird mixture of compatibility and changes (like GLib that
changed formatting along the way for MingW or newer msvcrt.dll supporting
C99 style formatting). Giving that Windows XP support was removed and
that Windows 7 seems to support C99 formatting I would say "ll" for
64 bit is good and also "I64" for ABI compatibility. So it looks like
there should be a way to tell GCC that the printf/ms_printf formatting
attribute should accepts also "ll" size modified for 64 bit integrals.

Frediano