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).
Jan
Actually, they can be deviated because they don't result in wrong code
being generated.
This, modulo the review comments received, is what most of the code
would look like if
they weren't, with the biggest pain point being that in many cases the
choice is either
the pattern with blocks for certain clauses or moving them in the
enclosing scope, which may
be several hundred lines above. If there's agreement on deviating them,
I can drop the patches
dealing with switches and do a v2 with just the other modifications.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
