More recvfile optimizations

Jeremy Allison jra at samba.org
Thu Apr 10 13:52:59 MDT 2014


On Thu, Apr 10, 2014 at 01:48:01PM +0200, Stefan (metze) Metzmacher wrote:
> Hi Jeremy,
> 
> here're some more smb2 recvfile optimizations.
> 
> The big TODO is an existing bug that we don't really check if the file
> handle
> supports recvfile, a named pipe doesn't support this and a print file
> may also doesn't support
> it. Currently it seems we don't have any good logic/checks to handle this.
> Do you have an idea how to do this properly for SMB1 and SMB2?

Well in smb1 we handle it in the main
socket read path using is_valid_writeX_buffer().

That checks the tid to make sure it's not an IPC
or print type.

Let me take a close look at your fix to do this for smb2,
I'll email back once I've gone over it.

> This is just for discussion yet until we discussed the above...

No problem, I'm not going to push *your* code until
you're happy (just my own :-) :-).

> >From e1d63f19517345138443055374493748dd42fcfe Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <metze at samba.org>
> Date: Thu, 5 Dec 2013 11:20:49 +0100
> Subject: [PATCH 1/8] s3:lib: use stack buffers in drain_socket() and
>  default_sys_recvfile()
> 
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  source3/lib/recvfile.c |   18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/source3/lib/recvfile.c b/source3/lib/recvfile.c
> index 72f4257..f1901d4 100644
> --- a/source3/lib/recvfile.c
> +++ b/source3/lib/recvfile.c
> @@ -51,7 +51,7 @@ static ssize_t default_sys_recvfile(int fromfd,
>  	size_t total = 0;
>  	size_t bufsize = MIN(TRANSFER_BUF_SIZE,count);
>  	size_t total_written = 0;
> -	char *buffer = NULL;
> +	char buffer[TRANSFER_BUF_SIZE];

Can't you use a C99 dynamic array here ? It's
likely that TRANSFER_BUF_SIZE might be bigger
than the passed in 'count' field. Might want
to use :

> +     char buffer[bufsize];

instead.

This patchset certainly looks good - we've
moving in the right direction !

Jeremy.


More information about the samba-technical mailing list