[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 at ffwll.ch>
>>
>> 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 at intel.com>
>> Cc: Jens Frederich <jfrederich at gmail.com>
>> Cc: Jon Nettleton <jon.nettleton at gmail.com>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: linux-staging at lists.linux.dev
>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> Cc: Helge Deller <deller at gmx.de>
>> Cc: Matthew Wilcox <willy at infradead.org>
>> Cc: Sam Ravnborg <sam at ravnborg.org>
>> Cc: Tetsuo Handa <penguin-kernel at i-love.sakura.ne.jp>
>> Cc: Zhen Lei <thunder.leizhen at huawei.com>
>> Cc: Alex Deucher <alexander.deucher at amd.com>
>> Cc: Xiyu Yang <xiyuyang19 at fudan.edu.cn>
>> Cc: linux-fbdev at vger.kernel.org
>> Cc: Zheyu Ma <zheyuma97 at gmail.com>
>> Cc: Guenter Roeck <linux at roeck-us.net>
>> Signed-off-by: Javier Martinez Canillas <javierm at redhat.com>
>> ---
>>
>> (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