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

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

Details

Reviewers: None
Commits: rP22621: Remove unused texture test helper function obsolete since rP8094 and rP14836…
Trac Tickets: #5294

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.Aug 6 2019, 9:22 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

Harbormaster completed remote builds in B8759: Diff 9247.Aug 6 2019, 9:28 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.

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

Harbormaster completed remote builds in B8761: Diff 9249.Aug 6 2019, 9:41 PM

This revision was not accepted when it landed; it landed in state Needs Review.Aug 6 2019, 11:00 PM

Closed by commit rP22621: Remove unused texture test helper function obsolete since rP8094 and rP14836… (authored by elexis). · Explain Why

This revision was automatically updated to reflect the committed changes.

elexis added a commit: rP22621: Remove unused texture test helper function obsolete since rP8094 and rP14836….