[PATCH] vfs_streams_xattr: Do not attempt to write empty attributetwice
Volker Lendecke
Volker.Lendecke at SerNet.DE
Thu Jun 13 13:50:31 MDT 2013
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
More information about the samba-technical
mailing list