From: Greg Kurz <groug@kaod.org>
To: Christian Schoenebeck <qemu_oss@crudebyte.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
qemu-devel@nongnu.org,
"Antonios Motakis" <antonios.motakis@huawei.com>
Subject: Re: [Qemu-devel] [PATCH v4 1/5] 9p: unsigned type for type, version, path
Date: Fri, 28 Jun 2019 14:06:24 +0200 [thread overview]
Message-ID: <20190628140624.1408c08c@bahia.lan> (raw)
In-Reply-To: <3608455.qB9dszzTOH@silver>
On Fri, 28 Jun 2019 13:42:43 +0200
Christian Schoenebeck <qemu_oss@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.motakis@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
next prev parent reply other threads:[~2019-06-28 13:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-26 18:57 [Qemu-devel] [PATCH v4 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
2019-06-26 18:25 ` [Qemu-devel] [PATCH v4 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
2019-06-27 16:12 ` Greg Kurz
2019-06-28 11:42 ` Christian Schoenebeck via Qemu-devel
2019-06-28 12:06 ` Greg Kurz [this message]
2019-06-26 18:30 ` [Qemu-devel] [PATCH v4 2/5] 9p: Treat multiple devices on one export as an error Christian Schoenebeck via Qemu-devel
2019-06-27 17:26 ` Greg Kurz
2019-06-28 12:36 ` Christian Schoenebeck via Qemu-devel
2019-06-28 12:47 ` Greg Kurz
2019-06-26 18:42 ` [Qemu-devel] [PATCH v4 3/5] 9p: Added virtfs option "remap_inodes" Christian Schoenebeck via Qemu-devel
2019-06-28 10:09 ` Greg Kurz
2019-06-28 13:47 ` Christian Schoenebeck via Qemu-devel
2019-06-28 14:23 ` Greg Kurz
2019-06-29 10:20 ` Christian Schoenebeck via Qemu-devel
2019-07-02 8:01 ` Greg Kurz
2019-06-26 18:46 ` [Qemu-devel] [PATCH v4 4/5] 9p: stat_to_qid: implement slow path Christian Schoenebeck via Qemu-devel
2019-06-28 10:21 ` Greg Kurz
2019-06-28 14:03 ` Christian Schoenebeck via Qemu-devel
2019-06-26 18:52 ` [Qemu-devel] [PATCH v4 5/5] 9p: Use variable length suffixes for inode remapping Christian Schoenebeck via Qemu-devel
2019-06-28 11:50 ` Greg Kurz
2019-06-28 14:56 ` Christian Schoenebeck via Qemu-devel
2019-06-29 11:01 ` Christian Schoenebeck via Qemu-devel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190628140624.1408c08c@bahia.lan \
--to=groug@kaod.org \
--cc=antonios.motakis@huawei.com \
--cc=berrange@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu_oss@crudebyte.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).