Re: [Xen-devel] [PATCH 2/5] libxl: make GC_FREE reachable in libxl_get_scheduler()

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2015-12-28 at 00:16 -0500, Joshua Otto wrote:
> Coverity CID 1343309
> 
> This patch preserves the multiple error paths in order to avoid
> meaninglessly assigning the ERROR_FAIL libxl error code to the
> return variable, which is of type libxl_scheduler.

Which makes me think that the existing code is bogus to return an error
code as a libxl_scheduler too, since that is not very different to the
bogus assignment.

The function ought to return LIBXL_SCHEDLER_UNKNOWN. However that might be
considered an API breakage (since at least xl checks for errors with < 0).

A _really_ correct libxl function API would take an output libxl_scheduler
pointer and return an int error code, but that is definitely an API change.

Given that a caller really ought to be handling LIBXL_SCHEDULER_UNKNOWN as
a return value, even if it is also written to expect negative error values
as well, so I reckon we can get away with changing the return to
SCHEDULER_UNKNOWN in the error case. Either the caller already handles
that, or it was already buggy in not doing so.

That would also allow us to move to a single error path, since you'd no
longer worry about clobbering.

Wei, Ian, any thoughts?

> Signed-off-by: Joshua Otto <jtotto@xxxxxxxxxxxx>
> ---
> Âtools/libxl/libxl.c | 2 +-
> Â1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index ca4679b..60a2509 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5590,8 +5590,8 @@ libxl_scheduler libxl_get_scheduler(libxl_ctx *ctx)
> ÂÂÂÂÂint r = xc_sched_id(ctx->xch, (int *)&sched);
> ÂÂÂÂÂif (r != 0) {
> ÂÂÂÂÂÂÂÂÂLOGE(ERROR, "getting current scheduler id");
> -ÂÂÂÂÂÂÂÂreturn ERROR_FAIL;
> ÂÂÂÂÂÂÂÂÂGC_FREE;
> +ÂÂÂÂÂÂÂÂreturn ERROR_FAIL;
> ÂÂÂÂÂ}
> ÂÂÂÂÂGC_FREE;
> ÂÂÂÂÂreturn sched;

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel