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

[Petri Latvala <petri.latvala@intel.com>]

On Wed, Nov 11, 2020 at 11:21:47AM +0100, Thomas Zimmermann wrote:
> 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@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.

Too many warnings already accumulated mostly, some of them false
positives.


> 
> > 
> > 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?

Actually I take that back, it's an established convention when you
pass data to functions, not so much in code like in your series where
the stuff is entirely contained within the function.

That leaves no benefits so option 2 looks real juicy here indeed.


-- 
Petri Latvala


> 
> 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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev