[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH 02/11] x86: move declarations to address MISRA C:2012 Rule 2.1



On Fri, 4 Aug 2023, Jan Beulich wrote:
> On 03.08.2023 16:22, Nicola Vetrini wrote:
> > On 03/08/2023 11:01, Jan Beulich wrote:
> >> On 03.08.2023 04:13, Stefano Stabellini wrote:
> >>> On Wed, 2 Aug 2023, Nicola Vetrini wrote:
> >>>> @@ -1169,8 +1170,6 @@ static void cf_check 
> >>>> irq_guest_eoi_timer_fn(void *data)
> >>>>
> >>>>      switch ( action->ack_type )
> >>>>      {
> >>>> -        cpumask_t *cpu_eoi_map;
> >>>
> >>> It is only used by case ACKTYPE_EOI so it can be moved there (with a 
> >>> new
> >>> block):
> >>>
> >>>
> >>>     case ACKTYPE_EOI:
> >>>     {
> >>>         cpumask_t *cpu_eoi_map = this_cpu(scratch_cpumask);
> >>>         cpumask_copy(cpu_eoi_map, action->cpu_eoi_map);
> >>>         spin_unlock_irq(&desc->lock);
> >>>         on_selected_cpus(cpu_eoi_map, set_eoi_ready, desc, 0);
> >>>         return;
> >>>     }
> >>>     }
> >>
> >> This pattern (two closing braces at the same level) is why switch scope
> >> variable declarations were introduced (at least as far as introductions
> >> by me go). If switch scope variables aren't okay (which I continue to
> >> consider questionable), then this stylistic aspect needs sorting first
> >> (if everyone else thinks the above style is okay - with the missing
> >> blank line inserted -, then so be it).
> > 
> > Actually, they can be deviated because they don't result in wrong code 
> > being generated.
> 
> Only later I recalled Andrew's intention to possibly make use of
> -ftrivial-auto-var-init. While, unlike I think he said, such declared
> variables aren't getting in the way of this (neither gcc nor clang
> warn about them), they also don't benefit from it (i.e. they'll be
> left uninitialized despite the command line option saying otherwise).
> IOW I think further consideration is going to be needed here.

Let me get this right. Are you saying that if we enable
-ftrivial-auto-var-init, due to a compiler limitation, variables
declared as follow:

  switch(var) {
      int a;
      char b;
      
      case ...

do not benefit from -ftrivial-auto-var-init ?

So if we moved the variable declarations elsewhere, in one of the two
following ways:

1)
  int a;
  int b;

  switch(var) {

2)
  switch(var) {

  case 1:
  {
      int a;
      int b;


Then -ftrivial-auto-var-init would take effect? If so, I think it is
worth a discussion on what to do there.

But either way right now I think it is fine to take the switch-related
patches out of this series.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.