[PATCH] vfs_streams_xattr: Do not attempt to write empty attributetwice

Jeremy Allison jra at samba.org
Mon Jun 17 12:00:59 MDT 2013


On Sat, Jun 15, 2013 at 09:17:00AM +0200, Volker Lendecke wrote:
> On Fri, Jun 14, 2013 at 12:07:42PM -0700, Jeremy Allison wrote:
> > On Thu, Jun 13, 2013 at 02:18:48PM -0700, Christof Schmitt wrote:
> > > On Thu, Jun 13, 2013 at 03:56:23PM -0400, Scott Lovenberg wrote:
> > > > On Thu, Jun 13, 2013 at 3:50 PM, Volker Lendecke
> > > > <Volker.Lendecke at sernet.de> wrote:
> > > > > That is probably true. But I think that this should be made
> > > > > obvious with braces. I find the operator precedence of || vs
> > > > > && less than obvious, and a bit of redundancy does not hurt
> > > > > here I guess.
> > > > >
> > > > > Looking at that whole routine, I think the general logic
> > > > > could be simplified quite a bit. (I know, much of that stuff
> > > > > is mine...)
> > > > >
> > > > > Volker
> > > > 
> > > > Yeah, I didn't even consider operator order until the second glance.
> > > > The binary will probably be the same either way and Volker's advise
> > > > makes the code much more readable.
> > > 
> > > Attached is an updated version of the patch with the braces added. My
> > > preference would be to fix the problem before restructuring the code,
> > > but i can still take a look.
> > > 
> > > Volker, how would you simplify this function?
> > > 
> > > -- 
> > > Christof Schmitt || IBM || SONAS System Development || Tucson, AZ
> > > christof.schmitt at us.ibm.com  ||  +1-520-799-2469  (T/L: 321-2469)
> > 
> > > >From 56dc5649d631dfab55c893bc2d4b99e61d13b691 Mon Sep 17 00:00:00 2001
> > > From: Christof Schmitt <christof.schmitt at us.ibm.com>
> > > Date: Wed, 12 Jun 2013 14:55:15 -0700
> > > Subject: [PATCH] vfs_streams_xattr: Do not attempt to write empty attribute twice
> > > 
> > > The create disposition FILE_OVERWRITE_IF is mapped to the flags
> > > O_CREAT|O_TRUNC. In vfs_streams_xattr, this triggers two calls to
> > > SMB_VFS_SETXATTR. The second can fail if O_EXCL is also set, resulting
> > > in an unnecessary error.
> > > 
> > > Merge the identical code to handle O_CREAT and O_TRUNC to avoid setting
> > > an empty attribute twice. Also add the flags parameter to the debug
> > > message.
> > > 
> > > Signed-off-by: Christof Schmitt <christof.schmitt at us.ibm.com>
> > 
> > Reviewed-by: Jeremy Allison <jra at samba.org>
> > 
> > Can I get a second Team review and I'll push to autobuild ?
> 
> +1

Pushed - thanks !

Jeremy.


More information about the samba-technical mailing list