Commit id 'f0469c61e' added the ability to load multiple mock libraries
by using the va_start() varable args marker; however, if something within
the VIR_TEST_PRELOAD fails, then control is returned to caller without
calling va_end. So add a couple more markers to indicat the need to call
va_end w/ the ap before returning.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
tests/qemucapsprobe.c | 2 +-
tests/testutils.c | 4 ++--
tests/testutils.h | 6 +++++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/tests/qemucapsprobe.c b/tests/qemucapsprobe.c
index 662c703..bca43b4 100644
--- a/tests/qemucapsprobe.c
+++ b/tests/qemucapsprobe.c
@@ -48,7 +48,7 @@ main(int argc, char **argv)
virThread thread;
virQEMUCapsPtr caps;
- VIR_TEST_PRELOAD(abs_builddir "/.libs/qemucapsprobemock.so");
+ VIR_TEST_PRELOAD(abs_builddir "/.libs/qemucapsprobemock.so", false, NULL);
if (argc != 2) {
fprintf(stderr, "%s QEMU_binary\n", argv[0]);
diff --git a/tests/testutils.c b/tests/testutils.c
index f455969..16fd3c6 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -904,11 +904,11 @@ int virTestMain(int argc,
virLogOutputPtr *outputs = NULL;
if (getenv("VIR_TEST_FILE_ACCESS"))
- VIR_TEST_PRELOAD(TEST_MOCK);
+ VIR_TEST_PRELOAD(TEST_MOCK, false, NULL);
va_start(ap, func);
while ((lib = va_arg(ap, const char *)))
- VIR_TEST_PRELOAD(lib);
+ VIR_TEST_PRELOAD(lib, true, ap);
va_end(ap);
progname = last_component(argv[0]);
diff --git a/tests/testutils.h b/tests/testutils.h
index c7f02e4..e4c602b 100644
--- a/tests/testutils.h
+++ b/tests/testutils.h
@@ -115,12 +115,14 @@ int virTestMain(int argc,
return virTestMain(argc, argv, func, NULL); \
}
-# define VIR_TEST_PRELOAD(lib) \
+# define VIR_TEST_PRELOAD(lib, haveap, _ap) \
do { \
const char *preload = getenv("LD_PRELOAD"); \
if (preload == NULL || strstr(preload, lib) == NULL) { \
char *newenv; \
if (!virFileIsExecutable(lib)) { \
+ if (haveap) \
+ va_end(_ap); \
perror(lib); \
return EXIT_FAILURE; \
} \
@@ -128,6 +130,8 @@ int virTestMain(int argc,
newenv = (char *) lib; \
} else if (virAsprintf(&newenv, "%s:%s", lib, preload) < 0) { \
perror("virAsprintf"); \
+ if (haveap) \
+ va_end(_ap); \
return EXIT_FAILURE; \
} \
setenv("LD_PRELOAD", newenv, 1); \
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jun 28, 2017 at 07:37:38 -0400, John Ferlan wrote:
> Commit id 'f0469c61e' added the ability to load multiple mock libraries
> by using the va_start() varable args marker; however, if something within
> the VIR_TEST_PRELOAD fails, then control is returned to caller without
> calling va_end. So add a couple more markers to indicat the need to call
> va_end w/ the ap before returning.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> tests/qemucapsprobe.c | 2 +-
> tests/testutils.c | 4 ++--
> tests/testutils.h | 6 +++++-
> 3 files changed, 8 insertions(+), 4 deletions(-)
[...]
> diff --git a/tests/testutils.h b/tests/testutils.h
> index c7f02e4..e4c602b 100644
> --- a/tests/testutils.h
> +++ b/tests/testutils.h
> @@ -115,12 +115,14 @@ int virTestMain(int argc,
> return virTestMain(argc, argv, func, NULL); \
> }
>
> -# define VIR_TEST_PRELOAD(lib) \
> +# define VIR_TEST_PRELOAD(lib, haveap, _ap) \
> do { \
> const char *preload = getenv("LD_PRELOAD"); \
> if (preload == NULL || strstr(preload, lib) == NULL) { \
> char *newenv; \
> if (!virFileIsExecutable(lib)) { \
> + if (haveap) \
> + va_end(_ap); \
> perror(lib); \
NACK, this is gross. Please make this macro jump to a label passed as
argument so that the caller can do cleanup.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06/28/2017 07:45 AM, Peter Krempa wrote:
> On Wed, Jun 28, 2017 at 07:37:38 -0400, John Ferlan wrote:
>> Commit id 'f0469c61e' added the ability to load multiple mock libraries
>> by using the va_start() varable args marker; however, if something within
>> the VIR_TEST_PRELOAD fails, then control is returned to caller without
>> calling va_end. So add a couple more markers to indicat the need to call
>> va_end w/ the ap before returning.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> tests/qemucapsprobe.c | 2 +-
>> tests/testutils.c | 4 ++--
>> tests/testutils.h | 6 +++++-
>> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> [...]
>
>> diff --git a/tests/testutils.h b/tests/testutils.h
>> index c7f02e4..e4c602b 100644
>> --- a/tests/testutils.h
>> +++ b/tests/testutils.h
>> @@ -115,12 +115,14 @@ int virTestMain(int argc,
>> return virTestMain(argc, argv, func, NULL); \
>> }
>>
>> -# define VIR_TEST_PRELOAD(lib) \
>> +# define VIR_TEST_PRELOAD(lib, haveap, _ap) \
>> do { \
>> const char *preload = getenv("LD_PRELOAD"); \
>> if (preload == NULL || strstr(preload, lib) == NULL) { \
>> char *newenv; \
>> if (!virFileIsExecutable(lib)) { \
>> + if (haveap) \
>> + va_end(_ap); \
>> perror(lib); \
>
> NACK, this is gross. Please make this macro jump to a label passed as
> argument so that the caller can do cleanup.
>
yuck - this was the cheaper/simpler approach. Not a fan of labels in the
macro... It would mean that virTestMain gets two labels (e.g. "error:"
and "va_error:") with a lot of code in between that returns EXIT_FAILURE
that could conceivably also "goto error;", but won't... I dunno, I just
find that worse. Similar for main in qemucapsprobe.c which would get an
error: label to return EXIT_FAILURE even though there's 5 other such
returns prior to that which don't goto error;
I'll just drop this and keep my local copy as is. If someone wants to be
more elegant, then have at it ;-)
Thanks for the quick reviews on these.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.