[Mesa-dev] [PATCH 3/3] glsl/cache/tests: Cast cache_get result to avoid compiler warning

[Aaron Watry]

On Tue, Nov 22, 2016 at 2:48 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On 11/22, Aaron Watry wrote:
>
>> disk_cache_get returns void*, but we were storing/comparing a char*.
>>
>> Signed-off-by: Aaron Watry <awatry at gmail.com>
>> ---
>> Note that this did, and still, segfaults for me when I actually run it...
>>
>
> Strange. It passes for me.
>

I guess that I should have clarified, this is also in the "building a
32-bit mesa on 64-bit host" scenario of my other two patches.

If I enable the shader cache on a 64-bit build, it works fine.

I'm not sure which scenario you're building for...

What I'm getting in my cross-compile scenario is:
/home/me/src/mesa/bin/test-driver: line 107: 10596 Segmentation fault
(core dumped) "$@" > $log_file 2>&1
FAIL: glsl/tests/cache-test

If I try to run the test directly, I get:
Error: Test 'disk_cache_create with no environment variables' failed:
Result=NULL, but expected something else.
Segmentation fault (core dumped)

Valgrind says:
==26330== Invalid read of size 4
==26330==    at 0x804A366: disk_cache_destroy (disk_cache.c:318)
==26330==    by 0x8048FA3: test_disk_cache_create (cache_test.c:143)
==26330==    by 0x8048FA3: main (cache_test.c:391)
==26330==  Address 0x8 is not stack'd, malloc'd or (recently) free'd

WIth that being said, I'm not seeing the warning that I thought I saw
before... I'm going to go back to the drawing board with this one and see
if I can address the segfault (looks like a failure to determine the user's
home directory for my environment, and I'm not sure why that's happening
for the 32-bit case).

The warnings that I thought I was getting before are no longer present, and
as you noted below should be allowed. It's possible I saw it when building
with clang instead of gcc, but I'm not sure anymore (I've been carrying
this patch locally for a while).

--Aaron




>
>
>> But at least the compiler is no longer complaining about the type
>> mismatch.
>> src/compiler/glsl/tests/cache_test.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/compiler/glsl/tests/cache_test.c
>> b/src/compiler/glsl/tests/cache_test.c
>> index 0ef05aa..c663e54 100644
>> --- a/src/compiler/glsl/tests/cache_test.c
>> +++ b/src/compiler/glsl/tests/cache_test.c
>> @@ -208,14 +208,14 @@ test_put_and_get(void)
>>    _mesa_sha1_compute(blob, sizeof(blob), blob_key);
>>
>>    /* Ensure that disk_cache_get returns nothing before anything is
>> added. */
>> -   result = disk_cache_get(cache, blob_key, &size);
>> +   result = (char*) disk_cache_get(cache, blob_key, &size);
>>
>
> void * is implicitly convertable to char *, so I think the code is fine
> as is. Maybe if I saw the compiler warning.
>
> "result" could be declared void * to begin with, and that would be fine.
> I just tested it, and it seems to work.
>
> Also, please use (char *) for casts and not (char*) in Mesa.
>

But, what I've also been seeing is sporadic warnings in the nir control
flow tests regarding signed/unsigned comparisons.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161122/4adab143/attachment.html>