Re: [XEN PATCH v2 1/4] build,include: rework shell script for headers++.chk

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.06.2022 12:31, Anthony PERARD wrote:
> On Wed, Jun 15, 2022 at 08:37:42AM +0200, Jan Beulich wrote:
>> On 14.06.2022 18:22, Anthony PERARD wrote:
>>> diff --git a/xen/include/Makefile b/xen/include/Makefile
>>> index 6d9bcc19b0..49c75a78f9 100644
>>> --- a/xen/include/Makefile
>>> +++ b/xen/include/Makefile
>>> @@ -158,13 +158,22 @@ define cmd_headerscxx_chk
>>>         touch $@.new;                                                     \
>>>         exit 0;                                                           \
>>>     fi;                                                                   \
>>> -   $(foreach i, $(filter %.h,$^),                                        \
>>> -       echo "#include "\"$(i)\"                                          \
>>> +   get_prereq() {                                                        \
>>> +       case $$1 in                                                       \
>>> +       $(foreach i, $(filter %.h,$^),                                    \
>>> +       $(if $($(patsubst $(srctree)/%,%,$(i))-prereq),                   \
>>> +           $(i)$(close)                                                  \
>>> +           echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \
>>> +                   -include c$(j))";;))                                  \
>>> +       esac;                                                             \
>>
>> If I'm reading this right (indentation looks to be a little misleading
>> and hence one needs to count parentheses) the case statement could (in
>> theory) remain without any "body". As per the command language spec I'm
>> looking at this (if it works in the first place) is an extension, and
>> formally there's always at least one label required. Since we aim to be
>> portable in such regards, I'd like to request that there be a final
>> otherwise empty *) line.
> 
> When looking at the shell grammar at [1], an empty body seems to be
> allowed. But I can add "*)" at the end for peace of mind.
> 
> [1] 
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10_02

Hmm, indeed. As opposed to

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_04

> As for misleading indentation, I've got my $EDITOR to show me matching
> parentheses, so I don't need to count them. But if I rewrite that
> function as following, would it be easier to follow?
> 
> +     get_prereq() {                                                        \
> +     case $$1 in                                                           \
> +     $(foreach i, $(filter %.h,$^),                                        \
> +         $(if $($(patsubst $(srctree)/%,%,$(i))-prereq),                   \
> +             $(i)$(close)                                                  \
> +             echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \
> +                     -include c$(j))";;))                                  \
> +     *) ;;                                                                 \
> +     esac;                                                                 \
> +     };                                                                    \

Hmm, not really, no (and it may be more obvious looking at the reply
context above): My primary concern is the use of hard tabs beyond the
leading one (which is uniform across all lines and hence doesn't alter
apparent alignment even with the + or > prefixed).

Jan