Re: [PATCH 1/4] Refactor request handling
On Mon, Mar 27, 2023 at 12:33:06PM +0200, Wouter Verhelst wrote:
> > ....
> > > +++ b/nbdsrv.h
> > > @@ -89,6 +89,14 @@ typedef struct {
> > > off_t startoff; /**< starting offset of this file */
> > > } FILE_INFO;
> > >
> > > +typedef struct {
> > > + struct nbd_request *req;
> > > + char *buf;
> > > + size_t buflen;
> > > + size_t current_offset;
> >
> > Should this be an off_t to begin with, or do we not care about 32-bit platforms?
>
> current_offset holds how much of the request we have already handled,
> not a file offset. In the case of simple replies (or structured replies
> with DF set), the reply is built into an allocated buffer, and in that
> case current_offset is used in pointer arithmetic (which implies it
> needs to be a size_t, or be at least cast to that).
>
> It's true that it isn't used in pointer arithmetics in the case of
> structured replies, since there we allocate a fresh buffer every time we
> get into find_read_buf. However, I still want to think of file_read_buf
> as a function returning a, possibly offset, pointer into a buffer even
> in that case, which also implies it should be a size_t rather than an
> off_t.
>
> Of course, in either case current_offset is *added* to a file offset in
> order to find the point where we should start reading, but that is only
> a secondary usage, not a primary one.
>
> But perhaps I'm missing something?
Until we have 64-bit extensions implemented (yes, I know I need to
post v3 of my work on that series), it doesn't matter. And even with
64-bit extensions, you are right that read and write commands bearing
a payload will still tend to be limited by ssize_t or smaller (even
size_t may be too big, depending on which underlying kernel syscalls
we are utilizing), where we aren't likely to overflow an offset in a
structured reply if we never allow a read request >4G in the first
place. So on re-reading my question and your response, I think you
are okay with size_t for current_offset.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Reply to: