On Mon, 2019-04-01 at 12:47 -0400, Cole Robinson wrote:
[...]
> @@ -758,6 +747,17 @@ testInfoClear(struct testInfo *info)
> virObjectUnref(info->qemuCaps);
> }
>
> +static int
> +testInfoSetPaths(struct testInfo *info, const char *suffix)
One argument per line.
> +{
> + if (virAsprintf(&info->infile, "%s/qemuxml2argvdata/%s.xml",
> + abs_srcdir, info->name) < 0 ||
> + virAsprintf(&info->outfile, "%s/qemuxml2argvdata/%s%s.args",
> + abs_srcdir, info->name, suffix ? suffix : "") < 0)
> + return -1;
This is very ugly. Please at least put curly braces around the if()
body and leave an empty line after it.
[...]
> @@ -883,11 +883,12 @@ mymain(void)
> do { \
> static struct testInfo info = { \
> .name = _name, \
> - .suffix = _suffix, \
> }; \
> if (testInfoSetArgs(&info, capslatest, \
> __VA_ARGS__, ARG_END) < 0) \
> return EXIT_FAILURE; \
> + if (testInfoSetPaths(&info, _suffix)) \
> + return EXIT_FAILURE; \
You should check whether testInfoSetPaths() returns a negative
value.
Also in xml2xml you print out a nice message with VIR_TEST_DEBUG()
when the function fails: you should do the same here as well.
With the above addressed,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
