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

Richard Sharpe realrichardsharpe at gmail.com
Tue May 15 07:45:50 MDT 2012

On Mon, May 14, 2012 at 10:58 PM, Michael Adam <obnox at samba.org> wrote:
> Hi Richard,
> 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:

 	if (ret) {
-		errno = saved_errno;
 		DEBUG(5, ("store_acl_blob_fsp: setting attr failed for file %s"
 			"with error %s\n",
-			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 :-)

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

Richard Sharpe

More information about the samba-technical mailing list