[PATCH] Fix smbclient regression not printing session setup anymore

[Jeremy Allison]

On Wed, Jun 07, 2017 at 08:07:20PM +0200, Stefan Metzmacher wrote:
> Hi Jeremy,
> 
> >> We used to pass 'inhdr' to smb_bytes_talloc_string() before,
> >> that way we got the alignment correct as the unicode characters
> >> are aligned to the beginning of the SMB request buffer.
> > 
> > Yep, I remember having to get that right a *LOOONG* time
> > ago - that's why an alarm went off in a dim reptillian
> > part of my brain when I first looked at the patch :-) :-).
> > 
> >> In smb_bytes_pull_str() is designed to pull the string
> >> just from the "bytes" part of the SMB request, which starts
> >> at an odd offset relative to the start of the complete SMB request
> >> buffer. I'm not 100% sure if that's also the case for chained requests,
> >> but at least samba aligns the wct field of subsequent requests to a
> >> 4-byte boundary, see smb_splice_chain() and smb1cli_req_chain_submit().
> >>
> >> The attached patch should improve this, but it would be better to have
> >> a regression test. I try to write a cmocka test for it with
> >> Andreas in the next days.
> > 
> > I'll review (and push if happy) the attached patch shortly.
> 
> Please only review and I'll sort out the details of the used
> BUG: reference with Andreas tomorrow.

OK, went through carefully and LGTM. RB+.

Can you think about whether we should remove the
bool align_odd parameter from internal_bytes_pull_str()
as well though (for a future patch). It's not currently
used at all and complicates the internal logic.

Cheers,

Jeremy.