Re: [Qemu-devel] [PATCH v4 1/5] 9p: unsigned type for type, version, path

[Christian Schoenebeck]

On Donnerstag, 27. Juni 2019 18:12:03 CEST Greg Kurz wrote:
> On Wed, 26 Jun 2019 20:25:55 +0200
> Christian Schoenebeck via Qemu-devel <address@hidden> wrote:
> > There is no need for signedness on these QID fields for 9p.
> > 
> > Signed-off-by: Antonios Motakis <address@hidden>
> 
> You should mention here the changes you made on top of Antonios
> original patch. Something like:
> 
> [CS: - also convert path
>      - adapted trace-events and donttouch_stat()]

Haven't seen that comment style in the git logs. Any example hash for that?

> > diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
> > index c0a0a4ab5d..6964756922 100644
> > --- a/hw/9pfs/trace-events
> > +++ b/hw/9pfs/trace-events
> > @@ -6,7 +6,7 @@ v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id
> > %d err %d"> 
> >  v9fs_version(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag
> >  %d id %d msize %d version %s" v9fs_version_return(uint16_t tag, uint8_t
> >  id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
> >  v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char*
> >  uname, char* aname) "tag %u id %u fid %d afid %d uname %s aname %s"> 
> > -v9fs_attach_return(uint16_t tag, uint8_t id, int8_t type, int32_t
> > version, int64_t path) "tag %d id %d type %d version %d path %"PRId64
> > +v9fs_attach_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t
> > version, uint64_t path) "tag %d id %d type %d version %d path %"PRId64
> I was expecting to see PRIu64 for an uint64_t but I now realize that %d
> seems to be used all over the place for unsigned types... :-\
> 
> At least, please fix the masks of the lines you're changing in this
> patch so that unsigned are passed to "u" or PRIu64. The rest of the
> mess can be fixed later in a followup.

If you don't mind I will restrict it to your latter suggestion for now, that 
is adjusting it using the short format specifiers e.g. "u", the rest would IMO 
be out of the scope of this patch series.

Too bad that no format specifier warnings are thrown on these.

Best regards,
Christian Schoenebeck