SMB3 Multi-Channel (Re: What I'm working on)

Stefan (metze) Metzmacher metze at samba.org
Thu Jul 24 11:06:07 MDT 2014


Hi Volker,

>> I also introduced a 'const char 'smbXsrv_connection_dbg(const struct
>> smbXsrv_connection *xconn)'
>> similar to fsp_fnum_dbg() and fsp_str_dbg().
>>
>> The commits with TODO/REVIEW are the chance which where added or modified.
> 
> Attached find my reviews. Almost all look good, I've just
> amended very few commit messages with questions. Also, I've
> added 3 related cleanup patches that you might want to
> review while you're looking at it.

Ok.

> The one with the sendfile destructor setting *pstatus is a
> bit ugly, but so is sendfile I guess..

Yep...

> On the one hand, there is this hunk:
> 
> -               if (write_data(fsp->conn->sconn->sock, buf, cur_read)
> -                   != cur_read) {
> -                       char addr[INET6_ADDRSTRLEN];
> +               ret = write_data(xconn->sconn->sock, buf, cur_read);
> +               if (ret != cur_read) {
> +                       int saved_errno = errno;
>
> but also

>                        to_write = MIN(SHORT_SEND_BUFSIZE, smb_maxcnt - nread);
> -                       if (write_data(fsp->conn->sconn->sock, buf, to_write)
> +                       if (write_data(xconn->sconn->sock, buf, to_write)
>                            != to_write) {

> So in some of the hunks we have the "never do function calls in
> expressions", convention pushed through, but not in all of them. Is
> there any reason for this?
>

I guess in some situations there where already a ret variable,
or in some I just forgot about it and did a strict string replacement.

I'll might change this to always ret, maybe as separate commit before
this one.

> Also, in some places I don't see errno is saved around the DEBUG message
> where it was not before. DEBUG itself can change the errno.

I'll have a look and fix this.

> TODO/REVIEW s3:smb2_server: talloc smbd_smb2_request as
> child of smbXsrv_connection

and

> TODO/REVIEW s3:smb2_server: remember smbXsrv_connection
> for each smbd_smb2_request

I'll extend the commit messages for these commits.

In the meantime I'll push patches up to:
s3:smbd: remove unused client_get_tcp_info()

Thanks very much for the review!
metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 246 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140724/4d73a50d/attachment.pgp>


More information about the samba-technical mailing list