Re: [PATCH v6 1/5] x86/boot: create a C bundle for 32 bit boot code and use it

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Oct 18, 2024 at 01:48:27PM +0100, Frediano Ziglio wrote:
> On Fri, Oct 18, 2024 at 12:41 PM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >
> > On Thu, Oct 17, 2024 at 02:31:19PM +0100, Frediano Ziglio wrote:
> > > +#define DECLARE_IMPORT(name) name = . + (__LINE__ * MULT)
> > > +
> > > +ENTRY(dummy_start)
> > >
> > >  SECTIONS
> > >  {
> > >    /* Merge code and data into one section. */
> > > -  .text : {
> > > +  .text TEXT_START : {
> > > +        /* Silence linker warning, we are not going to use it */
> > > +        dummy_start = .;
> > > +
> > > +        /* Declare below any symbol name needed.
> > > +         * Each symbol should be on its own line.
> > > +         * It looks like a tedious work but we make sure the things we 
> > > use.
> > > +         * Potentially they should be all variables. */
> >
> > The style is wrong for the opening and closing comment delimiters.
> >
> > I think it would be best if this was written in a more natural style.
> >
> > /*
> >  * Any symbols used should be declared below, this ensures which
> >  * symbols are visible to the 32bit C boot code.
> >  */
> >
> 
> But why to remove the "Potentially they should be all variables.".
> Surely something not written is more clear than something written, but
> on the other way it carries no information.

I'm not sure I understand why this is helpful: either they are
mandated to be only variables, and hence the "potentially" is wrong, or
they are not, in which case I don't see why spelling a desire for they
to be only variables is helpful if it's not a strict requirement.

Thanks, Roger.