On Fri, 28 Jun 2019 13:42:43 +0200
Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> 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 <qemu-devel@nongnu.org> wrote:
> > > There is no need for signedness on these QID fields for 9p.
> > >
> > > Signed-off-by: Antonios Motakis <antonios.mota...@huawei.com>
> >
> > 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?
>
$ git log | egrep '^[[:space:]]*\[' | head -15
[Commit message tweaked]
[Superfluous #include dropped]
[Comment reformatted to make checkpatch.pl happy, #include <dirent.h>
[monitor_is_qmp() tidied up to make checkpatch.pl happy,
[Header guard symbol tidied up, superfluous #include dropped, FIXME in
[sortcmdlist() cleaned up to make checkpatch.pl happy]
[Superfluous variable in monitor_data_destroy() eliminated, whitespace
[Superfluous variable in monitor_data_destroy() eliminated]
[Zero initialization of Monitor moved from monitor_data_init() to
[ ... ]
[ ... ]
[mreitz: Dropped superfluous printf from _filter_offsets, as suggested
[mreitz: Adjusted commit message as per John's proposal]
[mreitz: Moved from 250 to 256]
[AJB: fix conflicts with tests/vm: Port basevm to Python 3]
This is something you should do when re-posting someone else's patch
with modifications.
> > > 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.
>
Sure.
> Too bad that no format specifier warnings are thrown on these.
>
Yeah :-\
> Best regards,
> Christian Schoenebeck
