Re: [PATCH 2/2] Fix build with clang

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2016-03-11 at 16:36 +0100, Martin Kletzander wrote:
> Even though this is, technically speaking, a build-breaker fix,
> different version of this was posted before [1] and not accepted
> (although neither rejected), so I'm sending this as another way
> of approaching it.
> 
> [1] https://www.redhat.com/archives/libvir-list/2015-March/msg00203.html
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
>  tests/qemuxml2argvmock.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
> index 8426108b29ed..5667f34f1267 100644
> --- a/tests/qemuxml2argvmock.c
> +++ b/tests/qemuxml2argvmock.c
> @@ -62,6 +62,34 @@ virNumaNodeIsAvailable(int node)
>  {
>      return node >= 0 && node <= virNumaGetMaxNode();
>  }
> +
> +/* This is a copy-paste of the same function from src/util/virnuma.c.
> + * the reason for this is that some compilers might inline the
> + * function above (virNumaNodeIsAvailable) and hence mocking that
> + * function is pointless from our test suite's POV.  This is a
> + * (hopefully) temporary workaround until someone finds out how to
> + * disable inlining of exported functions with -O2 on clang.  The
> + * other option would be disabling inlining of that particular
> + * function which was proposed but did not come to a conclusion.
> + */

I'd rather go the __attribute__((noinline)) way, because I'm very
concerned that the two copies of virNumaNodesetIsAvailable() will
get out of sync at some point. If that happened, our tests will
continue to pass just fine even though the code shipped in the
library is potentially broken.

Of course if someone can come up with a proper fix, that's even
better :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]