[PATCH v6 3/5] ntoskrnl.exe: Implement volume information queries for device files.
Erich E. Hoover
erich.e.hoover at gmail.com
Thu Feb 11 12:17:35 CST 2021
On Wed, Feb 10, 2021 at 9:33 PM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> Taking a closer look since the last time I looked at this patch...
>
> > From: "Erich E. Hoover" <erich.e.hoover at gmail.com>
> > Subject: [PATCH v6 3/5] ntoskrnl.exe: Implement volume information queries for device files.
> > Message-Id: <CAEU2+vqrdb5u4fTQe80yUHBciFLwoF0RmiXMsFZ_s_wbndm+jQ at mail.gmail.com>
> > Date: Sat, 6 Feb 2021 11:25:22 -0700
> > ...
> > + if (out_size)
> > + {
> > + if (out_size > context->in_size)
> > + {
> > + if (!(out_buff = HeapAlloc( GetProcessHeap(), 0, out_size ))) return STATUS_NO_MEMORY;
> > + memcpy( out_buff, context->in_buff, context->in_size );
> > + to_free = context->in_buff;
> > + context->in_buff = out_buff;
> > + }
> > + else
> > + out_buff = context->in_buff;
> > + }
> > +
>
> This looks wrong. No data is ever passed in to this IRP, so it should
> probably just always allocate an output buffer, like dispatch_read().
This was copied from dispatch_ioctl under the assumption that some
future usage might need it, but you are right - according to the
documentation this IRP has no need for this. I'll go ahead and match
the behavior in dispatch_read.
> [There's probably an argument to reusing context.in_buff already
> allocated in dispatch_read() and here, but we'd need to store the
> capacity of the buffer somewhere. This patch will never reuse it, as
> in_size should always be 0.]
It looks to me like dispatch_read doesn't use context->in_buff, am I
missing something here? (Looks to me like it always does a heap
allocation.)
> > ...
> > + FILE_FS_VOLUME_INFORMATION *info = irp->AssociatedIrp.SystemBuffer;
> > + static WCHAR label[] = L"WineTestDriver";
>
> Missing "const"?
Yup, sorry about that.
> > + ULONG serial = 0xdeadbeef;
> > +
> > + if (length < sizeof(FILE_FS_VOLUME_INFORMATION))
> > + {
> > + ret = STATUS_INFO_LENGTH_MISMATCH;
> > + break;
> > + }
> > +
> > + info->VolumeCreationTime.QuadPart = 0; /* FIXME */
>
> "FIXME" seems a bit odd in a test like this, was that a copy/paste error?
Yes, that's a copy/paste error from patch 4.
> > ...
More information about the wine-devel
mailing list