Re: [PATCH for 1.2.6] vboxsnapshotxmltest: Don't write to a file in abs_srcdir

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Tue, Jul 01, 2014 at 03:28:49PM +0200, Michal Privoznik wrote:
> In the test, the snapshot XML is written into a file that's located
> under:
>
>  abs_srcdir/vboxsnapshotxmldata/testResult.vbox
>
> However, the abs_srcdir doesn't have to be necessarily writable. It
> should have been abs_builddir instead. Moreover, the label in the func
> creating the file is called 'fail' while it fulfils the duty of
> 'cleanup' label.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tests/vboxsnapshotxmltest.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
> index 7795688..85ef96a 100644
> --- a/tests/vboxsnapshotxmltest.c
> +++ b/tests/vboxsnapshotxmltest.c
> @@ -59,42 +59,47 @@ testCompareXMLtoXMLFiles(const char *xml)
>     char *pathResult = NULL;
>     int ret = -1;
>     virVBoxSnapshotConfMachinePtr machine = NULL;
> -    if (virAsprintf(&pathResult, "%s/vboxsnapshotxmldata/testResult.vbox",
> -                    abs_srcdir) < 0)
> +
> +    if (VIR_STRDUP(pathResult,
> +                   abs_builddir "/vboxsnapshotxmldata/testResult.vbox") < 0)
>         return -1;
>
> +    if (virFileMakePath(abs_builddir "/vboxsnapshotxmldata") < 0)
> +        goto cleanup;
> +
>     if (virtTestLoadFile(xml, &xmlData) < 0)
> -        goto fail;
> +        goto cleanup;
>
>     if (!(machine = virVBoxSnapshotConfLoadVboxFile(xml, (char*)"")))
> -        goto fail;
> +        goto cleanup;
>
>     if (virVBoxSnapshotConfSaveVboxFile(machine, pathResult) < 0)
> -        goto fail;
> +        goto cleanup;
>
>     if (virtTestLoadFile(pathResult, &actual) < 0)
> -        goto fail;
> +        goto cleanup;
>
>     if (unlink(pathResult) < 0)
> -        goto fail;
> +        goto cleanup;

This unlink should be done in the clean-up as well, otherwise rmdir()
won't remove the directory in case the test fails for example in
virtTestLoadFile().

ACK with that changed.

Martin

>     if (!(actual = testFilterXML(actual)))
> -        goto fail;
> +        goto cleanup;
>     if (!(xmlData = testFilterXML(xmlData)))
> -        goto fail;
> +        goto cleanup;
>
>     if (STRNEQ(actual, xmlData)) {
>         virtTestDifference(stderr, xmlData, actual);
> -        goto fail;
> +        goto cleanup;
>     }
>
>     ret = 0;
>
> - fail:
> + cleanup:
>     VIR_FREE(xmlData);
>     VIR_FREE(actual);
>     virVBoxSnapshotConfMachineFree(machine);
>     VIR_FREE(pathResult);
> +    rmdir(abs_builddir "/vboxsnapshotxmldata");
>
>     return ret;
> }
> --
> 1.8.5.5
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list