More recvfile optimizations

Stefan (metze) Metzmacher metze at samba.org
Thu Apr 10 15:05:14 MDT 2014


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.

I don't have a fix.

But we would need such check in is_smb2_recvfile_write(), correct?

>> 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.

So we decided to drop support for compilers without support dynamic arrays?

metze


More information about the samba-technical mailing list