[igt-dev] [PATCH v3 2/8] tests/fbdev: Map framebuffer in igt_fixture

[Thomas Zimmermann]

Hi,

thanks for looking at the patches.

Am 11.11.20 um 10:49 schrieb Petri Latvala:
> On Tue, Nov 10, 2020 at 08:50:56AM +0100, Thomas Zimmermann wrote:
>> The mapping of the framebuffer memory will be useful for read/write
>> tests. Move it into an igt_fixture block.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>>  tests/fbdev.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/fbdev.c b/tests/fbdev.c
>> index 46641fea..fc66210a 100644
>> --- a/tests/fbdev.c
>> +++ b/tests/fbdev.c
>> @@ -63,15 +63,10 @@ static void mode_tests(int fd)
>>  static void framebuffer_tests(int fd)
>>  {
>>  	struct fb_fix_screeninfo fix_info;
>> +	void *map;
> 
> After this patch, map is assigned to in an igt_fixture and read in a
> subtest. As igt_fixture uses longjmp(), the value of map when read
> depends on whether this is compiled with or without optimizations.

One thing I wondered about is that the tests are build with
-Wno-clobbered. What's the reason? I'd expect that the compiler would
have warned about this problem.

> 
> The options for such variables are:
> 
> 1) move it to global scope
> 2) make it volatile
> 3) make a struct data_d with the map (and in a later patch, buf as
>    well) variable and assign its members instead.
> 
> I think we prefer 3 generally.
> 

I would have used 2. What's the benefit of of using 3?

Best regards
Thomas

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer