[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Nov 2021 07:35:18 +0900
From: Dominique Martinet <asmadeus@...ewreck.org>
To: Christian Schoenebeck <linux_oss@...debyte.com>
Cc: Nikolay Kichukov <nikolay@...um.net>,
v9fs-developer@...ts.sourceforge.net, netdev@...r.kernel.org,
Eric Van Hensbergen <ericvh@...il.com>,
Latchesar Ionkov <lucho@...kov.net>,
Greg Kurz <groug@...d.org>, Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values
Christian Schoenebeck wrote on Mon, Nov 22, 2021 at 02:32:23PM +0100:
> I "think" this could be used for all 9p message types except for the listed
> former three, but I'll review the 9p specs more carefully before v4. For Tread
> and Twrite we already received the requested size, which just leaves Treaddir,
> which is however indeed tricky, because I don't think we have any info how
> many directory entries we could expect.
count in Treaddir is a number of bytes, not a number of entries -- so
it's perfect for this as well :)
> A simple compile time constant (e.g. one macro) could be used instead of this
> function. If you prefer a constant instead, I could go for it in v4 of course.
> For this 9p client I would recommend a function though, simply because this
> code has already seen some authors come and go over the years, so it might be
> worth the redundant code for future safety. But I'll adapt to what others
> think.
In this case a fallback constant seems simpler than a big switch like
you've done, but honestly I'm not fussy at this point -- if you work on
this you have the right to decide this kind of things in my opinion.
My worry with the snippet you listed is that you need to enumerate all
calls again, so if someday the protocol is extended it'll be a place to
forget adding new calls (although compiler warnings help with that),
whereas a fallback constant will always work as long as it's small
messages.
But really, as long as it's not horrible I'll take it :)
--
Dominique
Powered by blists - more mailing lists