On Thu, 5 Dec 2019 at 19:49, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, Dec 05, 2019 at 07:02:56PM +0000, Ard Biesheuvel wrote:
> > On Thu, 5 Dec 2019 at 18:56, Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > On Thu, Dec 05, 2019 at 04:42:48PM +0000, Ard Biesheuvel wrote:
>
> > > > +static int __init efi_earlycon_unmap_fb(void)
> > > > +{
> > > > + /* unmap the bootconsole fb unless keep_bootcon has left it enabled */
> > >
> > > > + if (efi_fb && !(earlycon_console->flags & CON_ENABLED))
> > >
> > > Isn't efi_fb test superfluous here?
> > >
> >
> > Well, it could be NULL so it is not superfluous. The initcall() is
> > always executed, even if you are not using earlycon=efifb at all.
>
> My point is that memunmap() is no-op for NULL pointer, isn't it?
>
Right. So I could check for earlycon_console != NULL instead, since
that is implied by efi_fb != NULL, but it could be NULL otherwise, in
which case I should not dereference it to get at the flags. But I need
two checks in any case, so I think this is OK.
> > > > + memunmap(efi_fb);
> > >
> > > > + return 0;
> > > > +}
> > > > +late_initcall(efi_earlycon_unmap_fb);
>
> > > I still think we need __exitcall() to clean up the stuff in any case.
> > >
> >
> > Why? As far as I can tell, exitcalls go straight into a /DISCARD/
> > section, and I really don't see the point of carrying dead code in
> > this file.
>
> Okay.
>
Thanks,
