[PATCH] Bug 11819 - The info type SMB2_SETINFO_FS is not been handled

Jeremy Allison jra at samba.org
Tue Apr 26 17:31:03 UTC 2016


On Tue, Apr 26, 2016 at 01:21:45PM +0300, Uri Simchoni wrote:
> On 04/26/2016 02:58 AM, Jeremy Allison wrote:
> > On Tue, Apr 19, 2016 at 10:18:14PM -0700, Partha Sarathi wrote:
> >> Thanks Jeremy for the comments, please find the updated patch where I have
> >> taken care of your comments and its a  "git format" patch with my
> >> 'Signed-off-by:'
> > 
> > Much better thanks Partha ! Here's a version that passes
> > the picky developer compiler flags (needed to cast
> > a %u to (unsigned int) in a printf format :-).
> > 
> > Can I get a second Team reviewer ?
> > 
> > Cheers,
> > 
> > 	Jeremy.
> > 
> The patch looks correct, but there appear to be some style/comment/debug
> level issues - most if not all originate at the original code, but if we
> touch it I think we should make it right. See attached patch on top of
> last patch - It fixes some debug messages, and also fiddles a bit with
> debug levels - I don't want to be religious about it, but there were
> just too many places where I disagreed with the debug level. I suppose
> they can be sqaushed together if it looks reasonable, or squashed with
> the original debug levels.

Thanks ! I didn't spot the DEBUG(0, issue - I'll squash these
on top of the original patch.

> On top of that, there's the issue of allocating the request array and
> freeing it afterwards:
> - Obviously in this particular case it has no use.
> - It looks like a pattern used in the SMB1 code to allow the request
> handler to return data.

Yep, this is an old pattern used in the trans2 code.

> - We can add it when we need it
> - Since it's SMB2-only code, I'm not sure the async code will work this
> way at all - maybe it will use subrequests to encapsulate
> info-class-specific return data.
> 
> So I propose to remove this extra allocation/freeing and add something
> suitable when the need arises.

OK, I'll fix that and send out another version for
review.

Thanks for the review !



More information about the samba-technical mailing list