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

[Richard Sharpe]

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:

 	}
 	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 :-)

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.

-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)