Listing public headers when out-of-tree build are involved becomes
more annoying where every path to every headers needs to start with
"$(srctree)/$(src)", or $(wildcard ) will not work. This means more
repetition. ( "$(srcdir)" is a shortcut for "$(srctree)/$(src)" )

This patch attempt to reduce the amount of duplication and make better
use of make's meta programming capability. The filters are now listed
in a variable and don't have to repeat the path to the headers files
as this is added later as needed.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v8:
    - add prefix "public-" to newly introduced macros.
    - make use of the new "$(srcdir)" shortcut.

 xen/include/Makefile | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 5a2b4c9f65fa..6e80ef276fd9 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -39,8 +39,8 @@ cppflags-$(CONFIG_X86)    += -m32
 
 endif
 
-public-$(CONFIG_X86) := $(wildcard $(src)/public/arch-x86/*.h $(src)/public/arch-x86/*/*.h)
-public-$(CONFIG_ARM) := $(wildcard $(src)/public/arch-arm/*.h $(src)/public/arch-arm/*/*.h)
+public-$(CONFIG_X86) := $(wildcard $(srcdir)/public/arch-x86/*.h $(srcdir)/public/arch-x86/*/*.h)
+public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/public/arch-arm/*/*.h)
 
 .PHONY: all
 all: $(addprefix $(obj)/,$(headers-y))
@@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
 
 all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
 
-PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y))
+public-hdrs-path := $(srcdir)/public
 
-PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h
-PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
+public-list-headers = $(wildcard $1/*.h $1/*/*.h)
+public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1))
+
+public-c99-headers := io/9pfs.h io/pvcalls.h
+public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y)
+public-ansi-headers := $(public-headers)
+
+public-headers-filter := dom0_ops.h arch-%
+public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers)
+
+PUBLIC_HEADERS := $(call public-filter-headers,public-headers)
+PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers)
+PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers))
 
 $(src)/public/io/9pfs.h-prereq := string
 $(src)/public/io/pvcalls.h-prereq := string
-- 
Anthony PERARD

On 25.11.2021 14:39, Anthony PERARD wrote:
> @@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>  
>  all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
>  
> -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y))
> +public-hdrs-path := $(srcdir)/public
>  
> -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h
> -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
> +public-list-headers = $(wildcard $1/*.h $1/*/*.h)
> +public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1))
> +
> +public-c99-headers := io/9pfs.h io/pvcalls.h
> +public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y)
> +public-ansi-headers := $(public-headers)

Personally I find it odd for public-c99-headers to be first in this group.
Further down, in the upper-case counterparts, you have it at the end (still
in context below).

> +public-headers-filter := dom0_ops.h arch-%

What is the criteria to be listed here vs ...

> +public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers)

... outside of that macro's expansion here? There's no other use of the
macro that I can spot, so its presence is puzzling me.

> +
> +PUBLIC_HEADERS := $(call public-filter-headers,public-headers)
> +PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers)
> +PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers))

While benign right now, wouldn't it be more consistent if
public-filter-headers was also applied in this last case? Or is
there a reason not to?

Jan

On Tue, Dec 21, 2021 at 03:07:52PM +0100, Jan Beulich wrote:
> On 25.11.2021 14:39, Anthony PERARD wrote:
> > @@ -81,10 +81,21 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
> >  
> >  all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
> >  
> > -PUBLIC_HEADERS := $(filter-out $(src)/public/arch-% $(src)/public/dom0_ops.h, $(wildcard $(src)/public/*.h $(src)/public/*/*.h) $(public-y))
> > +public-hdrs-path := $(srcdir)/public
> >  
> > -PUBLIC_C99_HEADERS := $(src)/public/io/9pfs.h $(src)/public/io/pvcalls.h
> > -PUBLIC_ANSI_HEADERS := $(filter-out $(src)/public/%ctl.h $(src)/public/xsm/% $(src)/public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
> > +public-list-headers = $(wildcard $1/*.h $1/*/*.h)
> > +public-filter-headers = $(filter-out $(addprefix $(public-hdrs-path)/,$($1-filter)), $($1))
> > +
> > +public-c99-headers := io/9pfs.h io/pvcalls.h
> > +public-headers := $(call public-list-headers,$(public-hdrs-path)) $(public-y)
> > +public-ansi-headers := $(public-headers)
> 
> Personally I find it odd for public-c99-headers to be first in this group.
> Further down, in the upper-case counterparts, you have it at the end (still
> in context below).
> 
> > +public-headers-filter := dom0_ops.h arch-%
> 
> What is the criteria to be listed here vs ...
> 
> > +public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) $(public-c99-headers)
> 
> ... outside of that macro's expansion here? There's no other use of the
> macro that I can spot, so its presence is puzzling me.

The "%-filter" macro are used by the macro "public-filter-headers",
which takes "$(%)" and filter-out "$(%-filter)".

Currently, PUBLIC_ANSI_HEADERS is PUBLIC_HEADERS with headers
filtered-out.
And PUBLIC_HEADERS is public-y with headers filtered-out.

In my patch, I derive PUBLIC_ANSI_HEADERS from public-y, so I also need
to use the filter used to generate PUBLIC_HEADERS. So
$(public-headers-filter) is both used on its own and in the ansi-filter.

> > +PUBLIC_HEADERS := $(call public-filter-headers,public-headers)
> > +PUBLIC_ANSI_HEADERS := $(call public-filter-headers,public-ansi-headers)
> > +PUBLIC_C99_HEADERS := $(addprefix $(public-hdrs-path)/, $(public-c99-headers))
> 
> While benign right now, wouldn't it be more consistent if
> public-filter-headers was also applied in this last case? Or is
> there a reason not to?

It wouldn't work at the moment because $(public-c99-headers) aren't
prefix with "$(public-hdrs-path)" like $(public-headers). This is
because I'm using $(public-c99-headers) in
$(public-ansi-headers-filter).

Maybe I should just had the prefix to the c99-headers and removing the
prefix when using it in the filter, something like that:

    public-c99-headers := $(addprefix $(public-hdrs-path)/,io/9pfs.h io/pvcalls.h)
    public-ansi-headers-filter := %ctl.h xsm/% %hvm/save.h $(public-headers-filter) \
        $(patsubst $(public-hdrs-path)/%,%,$(public-c99-headers))

then I thin that would work:
    PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers)

I could put an empty "public-c99-headers-filter :=" somewhere, just in
case one is looking for it later.

Thanks,

-- 
Anthony PERARD