[libvirt] [PATCH 05/23] qemu_conf: split out virQEMUDriverConfigLoadSWTPMEntry

[John Ferlan]

On 1/15/19 8:23 AM, Ján Tomko wrote:
> Split out parts of the config parsing code to make
> the parent function easier to read.
> 
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/qemu/qemu_conf.c | 36 +++++++++++++++++++++++++-----------
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 837bff6b30..325e5ccfd5 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -423,6 +423,30 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
>  }
>  
>  
> +static int
> +virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg,
> +                                  virConfPtr conf)
> +{
> +    char *swtpm_user = NULL, *swtpm_group = NULL;

Some would note these should be on separate lines, but I'll note that
since we have it, these should be:

    VIR_AUTOPTR(char *) swtpm_user = NULL;
    VIR_AUTOPTR(char *) swtpm_group = NULL;

> +    int ret = -1;
> +
> +    if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0)
> +        goto cleanup;
> +    if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0)
> +        goto cleanup;
> +
> +    if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0)
> +        goto cleanup;
> +    if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(swtpm_user);
> +    VIR_FREE(swtpm_group);

Thus freeing you of needing this and perhaps changing your logic above.

I "understand" the "norm" is to just purely copy and then have another
patch, but I really believe in this case it's so obvious that a separate
patch isn't required especially since VIR_AUTOFREE is more commonly used.

> +    return ret;
> +}
> +

one more blank line.  If you feel you must create two patches out of
this one patch, then go ahead, I trust that you can do that, but I think
we really should use the VIR_AUTOPTR more now...

BTW: I have a feeling VIR_AUTOPTR and 2 empty lines will become a
recurring theme...

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]