[libvirt] [PATCH 1/2] bhyve: Separate out checks from virBhyveProbeCaps

[John Ferlan]

On 08/27/2016 09:11 AM, Roman Bogorodskiy wrote:
> From: Fabian Freyer <fabian.freyer at physik.tu-berlin.de>
> 
> At the moment this is just one check, but separating this out into a
> separate function makes checks more modular, allowing for more readable
> code once more checks are added. This also makes checks more easily
> testable.
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy at gmail.com>
> ---
>  src/bhyve/bhyve_capabilities.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_capabilities.c b/src/bhyve/bhyve_capabilities.c
> index 10c33b9..be68e51 100644
> --- a/src/bhyve/bhyve_capabilities.c
> +++ b/src/bhyve/bhyve_capabilities.c
> @@ -168,19 +168,13 @@ virBhyveProbeGrubCaps(virBhyveGrubCapsFlags *caps)
>      return ret;
>  }
>  
> -int
> -virBhyveProbeCaps(unsigned int *caps)
> +static int
> +bhyveProbeCapsRTC_UTC(unsigned int *caps, char *binary)
>  {
> -    char *binary, *help;
> +    char *help;
>      virCommandPtr cmd = NULL;
>      int ret = 0, exit;
>  
> -    binary = virFindFileInPath("bhyve");
> -    if (binary == NULL)
> -        goto out;
> -    if (!virFileIsExecutable(binary))
> -        goto out;
> -
>      cmd = virCommandNew(binary);
>      virCommandAddArg(cmd, "-h");
>      virCommandSetErrorBuffer(cmd, &help);
> @@ -195,6 +189,25 @@ virBhyveProbeCaps(unsigned int *caps)
>   out:
>      VIR_FREE(help);
>      virCommandFree(cmd);
> +    return ret;
> +}
> +
> +int
> +virBhyveProbeCaps(unsigned int *caps)
> +{
> +    char *binary;
> +    int ret = 0;
> +
> +    binary = virFindFileInPath("bhyve");
> +    if (binary == NULL)
> +        goto out;

The above could be optimized as:

    if (!(binary = virFindFileInPath("bhyve")))
        return 0;

> +    if (!virFileIsExecutable(binary))
> +        goto out;
> +
> +    if ((ret = bhyveProbeCapsRTC_UTC(caps, binary)))
> +        goto out;

Why even bother with the if construct - in either case you're going to out.

ACK with slight adjustments - seems reasonable to me.

John
> +
> + out:
>      VIR_FREE(binary);
>      return ret;
>  }
>