Re: [PATCH v5 7/7] fbdev: Make registered_fb[] private to fbmem.c

[Guenter Roeck]

On 5/11/22 10:00, Sam Ravnborg wrote:
> Hi Javier.
>
> On Wed, May 11, 2022 at 01:32:30PM +0200, Javier Martinez Canillas wrote:
> > From: Daniel Vetter <daniel.vetter@xxxxxxxx>
> >
> > Well except when the olpc dcon fbdev driver is enabled, that thing
> > digs around in there in rather unfixable ways.
> >
> > Cc oldc_dcon maintainers as fyi.
>
> Another way to fix this is to mark FB_OLPC_DCON and add a TODO entry to
> fix this. We are really not supposed to carry such special code around
> to keep staging working.
>
> I know this may not be a popular viewpoint, but just look at the ugly
> workarounds required here.
>
> 	Sam
>
>
> > v2: I typoed the config name (0day)
> >
> > Cc: kernel test robot <lkp@xxxxxxxxx>
> > Cc: Jens Frederich <jfrederich@xxxxxxxxx>
> > Cc: Jon Nettleton <jon.nettleton@xxxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: linux-staging@xxxxxxxxxxxxxxx
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> > Cc: Daniel Vetter <daniel@xxxxxxxx>
> > Cc: Helge Deller <deller@xxxxxx>
> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> > Cc: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
> > Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> > Cc: Xiyu Yang <xiyuyang19@xxxxxxxxxxxx>
> > Cc: linux-fbdev@xxxxxxxxxxxxxxx
> > Cc: Zheyu Ma <zheyuma97@xxxxxxxxx>
> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> > ---
> >
> > (no changes since v1)
> >
> >   drivers/video/fbdev/core/fbmem.c | 8 ++++++--
> >   include/linux/fb.h               | 7 +++----
> >   2 files changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
> > index 265efa189bcc..6cab5f4c1fb3 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -50,10 +50,14 @@
> >   static DEFINE_MUTEX(registration_lock);
> >   
> >   struct fb_info *registered_fb[FB_MAX] __read_mostly;
> > -EXPORT_SYMBOL(registered_fb);
> > -
> >   int num_registered_fb __read_mostly;
> > +#if IS_ENABLED(CONFIG_FB_OLPC_DCON)
> > +EXPORT_SYMBOL(registered_fb);
> >   EXPORT_SYMBOL(num_registered_fb);
> > +#endif
>
> It is stuff like this I refer to as "ugly" in the comment above.

My "solution" for that kind of thing is to use a namespace,
such as

EXPORT_SYMBOL_NS(registered_fb, FB_OLPC_DCON);
EXPORT_SYMBOL_NS(num_registered_fb, FB_OLPC_DCON);

and import it from the offending code. That avoids ifdefs
while at the same time limiting the use of the symbols
to the expected scope. Of course that could be abused but
that abuse would be obvious.

Guenter