What's wrong with this piece of code from source3/modules/vfs_acl_xattr.c around like 126 or so?

[Michael Adam]

Hi Richard.

Richard Sharpe wrote:
> On Mon, May 14, 2012 at 10:58 PM, Michael Adam <obnox at samba.org> wrote:
> > I guess the point that the code is trying to make is that
> > it wants to protect the original errno from being overwritten
> > by intermediate actions like unbecome_root, debugging, etc.
> > unbecome_root does quite a lot of operations..
> >
> > To do that consequently we could change that according
> > to the attached patch. Does that make it more clear
> > and maybe more correct? With this patch, the errno is only
> > reset to the saved value directly before the return.
> 
> That's very funny:
> 
>  	}
>  	unbecome_root();
>  	if (ret) {
> -		errno = saved_errno;
>  		DEBUG(5, ("store_acl_blob_fsp: setting attr failed for file %s"
>  			"with error %s\n",
>  			fsp_str_dbg(fsp),
> -			strerror(errno) ));
> -		return map_nt_error_from_unix(errno);
> +			strerror(saved_errno) ));
> +		return map_nt_error_from_unix(saved_errno);
> +		errno = saved_errno;
>  	}
>  	return NT_STATUS_OK;
>  }
> 
> I suspect that a good compiler would say that the statement after the
> return has no effect :-)

Gosh, that is embarassing... and funny indeed ;)
Seem to have written it in a hurry.
Of course the errno = saved_errno and the return need to be
reversed.

> The intent is close. There is an error in the caller as well. It
> discards the return code. I will fix the problem in master and commit
> it.

Ok. Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120515/d6c33811/attachment.pgp>