Page MenuHomeWildfire Games

Fix test_tex.h gcc compiler warning about memset call against a class without trivial copy assignment (Wclass-memaccess)
ClosedPublic

Authored by elexis on Tue, Aug 6, 9:22 PM.

Details

Summary

The following compiler warning is emitted since gcc 7:

In file included from ../../../source/lib/res/graphics/tests/test_tex.cpp:17:
/path/to/0ad/source/lib/res/graphics/tests/test_tex.h: In member function ‘void TestTex::generate_encode_decode_compare(size_t, size_t, size_t, size_t, const OsPath&)’:
/path/to/0ad/source/lib/res/graphics/tests/test_tex.h:49:27: warning: ‘void* memset(void*, int, size_t)’ clearing an object of type ‘struct Tex’ with no trivial copy-assignment; use assignment or value-initialization instead [-Wclass-memaccess]
    memset(&t, 0, sizeof(t));
                           ^
In file included from /path/to/0ad/source/lib/res/graphics/tests/test_tex.h:25,
                 from ../../../source/lib/res/graphics/tests/test_tex.cpp:17:
../../../source/lib/tex/tex.h:209:8: note: ‘struct Tex’ declared here
 struct Tex
        ^~~
Test Plan

Phrase the testcase so that it best reflects the intended unit test.
There are several alternative approaches. One would be t = Tex(), but in theory this doesn't necessarily imply that the memory is completely zeroed out and that some dangling pointer elsewhere might still read from it, which the memset did rule out afaics. I guess calling the destructor explicitly doesn't guarantee that either, just that it's more explicit as to what was meant before.

Another possibility is to split the scope into two scopes and use two different Tex variables. But that would invalidate the comment // Once the Tex created here goes out of scope, the DynArray should be freed.
I don't know if the comment is valid to begin with, or whether rather the function scope was meant.

Another possibility would be to call the free() function of the struct.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Tue, Aug 6, 9:22 PM
Vulcan added a comment.Tue, Aug 6, 9:28 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/res/graphics/tests/test_tex.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/lib/res/graphics/tests/test_tex.h
|  29| class·TestTex·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestTex:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/324/display/redirect

elexis updated this revision to Diff 9249.Tue, Aug 6, 9:34 PM

Actually it looks cleaner to not reuse a struct instance after having called the destructor without calling its constructor again, even if it happens to work.
Use a second class instance, and destroy the fist one regardless to express the intent that memset carried.

Vulcan added a comment.Tue, Aug 6, 9:41 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/lib/res/graphics/tests/test_tex.h
|   1| /*·Copyright·(C)·2010·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2010"

source/lib/res/graphics/tests/test_tex.h
|  29| class·TestTex·:·public·CxxTest::TestSuite
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classTestTex:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/326/display/redirect

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Aug 6, 11:00 PM
This revision was automatically updated to reflect the committed changes.