[PATCH 0/2] FSCTL_SRV_COPYCHUNK_WRITE
David Disseldorp
ddiss at suse.de
Thu Oct 17 22:15:36 MDT 2013
Thanks for the review Jeremy...
Jeremy Allison <jra at samba.org> wrote:
>On Thu, Oct 17, 2013 at 12:36:59PM +0200, David Disseldorp wrote:
>> This trivial patch series adds support for the
>FSCTL_SRV_COPYCHUNK_WRITE
>> ioctl. It differs slightly from FSCTL_SRV_COPYCHUNK, in that it
>permits
>> copy-chunk requests against targets where the destination file handle
>> does not have read access.
>
>Code works but I have two requests for a change.
>
>Firstly, "bool dst_needs_rw" isn't clear enough.
>It doesn't really articulate what the options
>are here - they are really READ|WRITE, or WRITE.
>
>There ought to be a better way to articulate
>that (sorry, can't immediately think of that
>now).
How about passing through the ioctl value and putting the comment next to the read access check?
I'll repost with this logic.
>Second point:
>
>+ case FSCTL_SRV_COPYCHUNK_WRITE:
>+ /*
>+ * [MS-SMB2] 2.2.31
>+ * FSCTL_SRV_COPYCHUNK is issued when a handle has
>+ * FILE_READ_DATA and FILE_WRITE_DATA access to the
>file;
>+ * FSCTL_SRV_COPYCHUNK_WRITE is issued when a handle
>only has
>+ * FILE_WRITE_DATA access.
>+ */
>+ cc_dst_needs_rw = false;
> case FSCTL_SRV_COPYCHUNK:
>
>is a fallthough on a switch statement without at least
>a comment showing this is intended. That's a horror and
>an abomination :-) :-).
Fair enough, will fix :-)
More information about the samba-technical
mailing list