Re: [Xen-devel] [PATCH v5 3/4] xen/x86: use SYMBOL when required

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 8 Jan 2019, Jan Beulich wrote:
> >>> On 07.01.19 at 20:33, <sstabellini@xxxxxxxxxx> wrote:
> > On Mon, 7 Jan 2019, Jan Beulich wrote:
> >> >>> On 03.01.19 at 20:19, <sstabellini@xxxxxxxxxx> wrote:
> >> > --- a/xen/arch/x86/alternative.c
> >> > +++ b/xen/arch/x86/alternative.c
> >> > @@ -194,7 +194,7 @@ void init_or_livepatch apply_alternatives(struct 
> >> > alt_instr *start,
> >> >       * So be careful if you want to change the scan order to any other
> >> >       * order.
> >> >       */
> >> > -    for ( a = base = start; a < end; a++ )
> >> > +    for ( a = base = start; (unsigned long)a < SYMBOL(end); a++ )
> >> 
> >> As said before, imo SYMBOL() should only eve be applied _directly_
> >> to one of the (commonly linker script generated) symbols listed above,
> >> never to any derivatives.
> > 
> > This is the most problematic case for doing that. This issue also
> > highlights one of the problems mentioned in the thread about returning
> > unsigned long or native type from SYMBOL.
> > 
> > To address your comment, I could do the following:
> > 
> >         apply_alternatives((struct alt_instr *)SYMBOL(__alt_instructions),
> >                            (struct alt_instr 
> > *)SYMBOL(__alt_instructions_end));
> > 
> > But then, we would still have two pointers pointing to different objects
> > being compared in the implementation of apply_alternatives: `a' and
> > `end' would be pointers pointing to __alt_instructions and
> > __alt_instructions_end respectively. The MISRA-C violation would not be
> > addressed, I think. The same thing would happen here, and in many other
> > instances, if we switched SYMBOL to returning the native pointer type.
> > 
> > My preference would be to keep the code as-is in this patch and add an
> > in-code comment in apply_alternatives to explain the behavior.
> 
> See my other reply (on the v4 thread): No matter what you do with
> SYMBOL(), it is just a hiding mechanism. With that, it should be of
> no interest to the validation where exactly the hiding occurs. Any
> fully C language compliant solution can only involve replacement of
> the "end" symbols by expressions evaluating to the size of a
> particular object, _without_ subtracting start from end addresses.

I am not sure if what you say is is really what MISRA-C compliance
entails. But I'll send an update with SYMBOL returning the native type.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel