Reset the whole stack in testutils

[libvirt] [PATCH] Reset the whole stack in testutils

Posted by Martin Kletzander 6 years, 10 months ago

The memset() was resetting only 30 bytes in the array (size of the
array), but it is array of pointers.  Since it is a static array,
let's just reset it by its size.

Found by gcc-7.1:

  testutils.c: In function 'virTestRun':
  testutils.c:243:13: error: 'memset' used with length equal to number
  of elements without multiplication by element size [-Werror=memset-elt-size]
    memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack));
    ^~~~~~

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
Pushed as trivial an also under the build-breaker rule.

I'm still getting one more error in config.h that probably needs
fixing in autoconf:

../config.h:2994:48: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
             || (defined _FORTIFY_SOURCE && 0 < _FORTIFY_SOURCE \

Let me know if you know how I could help with that.

 tests/testutils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testutils.c b/tests/testutils.c
index 4fb2338bb1de..4b8cf79ef939 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -240,7 +240,7 @@ virTestRun(const char *title,
         for (i = start; i < end; i++) {
             bool missingFail = false;
 # ifdef TEST_OOM_TRACE
-            memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack));
+            memset(testAllocStack, 0, sizeof(testAllocStack));
             ntestAllocStack = 0;
 # endif
             virAllocTestOOM(i + 1, 1);
--
2.13.0

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

Re: [libvirt] [PATCH] Reset the whole stack in testutils

Posted by Daniel P. Berrange 6 years, 10 months ago

On Mon, Jun 05, 2017 at 10:01:10AM +0200, Martin Kletzander wrote:
> The memset() was resetting only 30 bytes in the array (size of the
> array), but it is array of pointers.  Since it is a static array,
> let's just reset it by its size.
> 
> Found by gcc-7.1:
> 
>   testutils.c: In function 'virTestRun':
>   testutils.c:243:13: error: 'memset' used with length equal to number
>   of elements without multiplication by element size [-Werror=memset-elt-size]
>     memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack));
>     ^~~~~~
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> Pushed as trivial an also under the build-breaker rule.
> 
> I'm still getting one more error in config.h that probably needs
> fixing in autoconf:
> 
> ../config.h:2994:48: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
>              || (defined _FORTIFY_SOURCE && 0 < _FORTIFY_SOURCE \
> 
> Let me know if you know how I could help with that.

Looks like that comes from gnulib actually, so something to reoprt there
I guess

$ git grep FORTIFY_SOURCE | grep defined
m4/extern-inline.m4:            || (defined _FORTIFY_SOURCE && 0 < _FORTIFY_SOURCE \



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH] Reset the whole stack in testutils

Posted by Martin Kletzander 6 years, 10 months ago

On Mon, Jun 05, 2017 at 10:02:17AM +0100, Daniel P. Berrange wrote:
>On Mon, Jun 05, 2017 at 10:01:10AM +0200, Martin Kletzander wrote:
>> The memset() was resetting only 30 bytes in the array (size of the
>> array), but it is array of pointers.  Since it is a static array,
>> let's just reset it by its size.
>>
>> Found by gcc-7.1:
>>
>>   testutils.c: In function 'virTestRun':
>>   testutils.c:243:13: error: 'memset' used with length equal to number
>>   of elements without multiplication by element size [-Werror=memset-elt-size]
>>     memset(testAllocStack, 0, ARRAY_CARDINALITY(testAllocStack));
>>     ^~~~~~
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>> Pushed as trivial an also under the build-breaker rule.
>>
>> I'm still getting one more error in config.h that probably needs
>> fixing in autoconf:
>>
>> ../config.h:2994:48: warning: this use of "defined" may not be portable [-Wexpansion-to-defined]
>>              || (defined _FORTIFY_SOURCE && 0 < _FORTIFY_SOURCE \
>>
>> Let me know if you know how I could help with that.
>
>Looks like that comes from gnulib actually, so something to reoprt there
>I guess
>
>$ git grep FORTIFY_SOURCE | grep defined
>m4/extern-inline.m4:            || (defined _FORTIFY_SOURCE && 0 < _FORTIFY_SOURCE \
>

You're absolutely right, I didn't realize that's where it could come
from.  Patch posted on the bug-gnulib list (e.g. if Eric wants to have a
look at that) =)

Have a nice day,
Martin

>
>
>Regards,
>Daniel
>--
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list