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

[Volker Lendecke]

On Thu, Jun 13, 2013 at 11:26:35AM -0700, Christof Schmitt wrote:
> On Thu, Jun 13, 2013 at 05:11:07AM +0000, Jim Brown wrote:
> > You are missing a set of parentheses here:
> > 
> > -	if (!NT_STATUS_IS_OK(status)) {
> > +	if (!NT_STATUS_IS_OK(status) && (flags & O_CREAT) ||
> > +	    (flags & O_TRUNC)) {
> > 
> > It should be:
> > 
> > -	if (!NT_STATUS_IS_OK(status)) {
> > +	if (!NT_STATUS_IS_OK(status) && ((flags & O_CREAT) ||
> > +	    (flags & O_TRUNC))) {
> 
> The logic before that patch is:
> if (!NT_STATUS_IS_OK(status)) {
>                 if (flags & O_CREAT) {
> 			// create empty attributes
> 		}
> }
> if (flags & O_TRUNC) {
> 			// create empty attributes
> }
> 
> Your proposal would change that logic to move the O_TRUNC case inside
> the !NT_STATUS_IS_OK case. Since !NT_STATUS_IS_OK indicates that the
> attribute does not exist, this would only allow truncate for
> non-existing attributes.

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

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de