[Top][All Lists]

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

Re: [Qemu-devel] [PATCH 01/47] rules.mak: New logical functions

From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 01/47] rules.mak: New logical functions
Date: Fri, 13 Sep 2013 14:43:34 +0100

On 25 August 2013 23:58, Ákos Kovács <address@hidden> wrote:
> lnot, land, lor, lif, eq, ne, isempty, notempty functions added
> Example usage:
>     obj-$(call lor,$(CONFIG_LINUX),$(CONFIG_BSD)) += feature.o
> Signed-off-by: Ákos Kovács <address@hidden>

Hi; I like the general idea here but I think there
are some issues with your implementation.

> ---
>  rules.mak |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> diff --git a/rules.mak b/rules.mak
> index 4499745..7e8e3bd 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -106,6 +106,22 @@ clean: clean-timestamp
>  obj := .
>  old-nested-dirs :=
> +# Logical functions

I think we should be clear here about the input
and output ranges of these functions, ie that
the inputs should always be "y", "n" or "" (with
either "n" or the empty string being false).

> +lnot = $(if $(subst n,,$1),n,y)
> +
> +land-yy = y
> +land = $(land-$1$2)

This means that $(call land,y,n) would expand
to the empty string, not 'y' or 'n', right?
I think we should make all these functions always
expand to either 'y' or 'n'.

land = $(if $(findstring yy,$1$2),y,n)

will do this.

> +lor = $(if $(subst $2,,$1)$(subst $1,,$2),n,y)

This can't be correct for both 'lor' and 'eq'.
In fact it works as 'eq'. A working lor would be:

lor = $(if $(findstring y,$1$2),y,n)

> +lif = $(if $(subst n,,$1),$2,$3)
> +
> +eq = $(if $(subst $2,,$1)$(subst $1,,$2),n,y)
> +ne = $(if $(subst $2,,$1)$(subst $1,,$2),y,n)

These give the wrong answer for comparisons
of 'n' with ''. Working versions:

eq = $(if $(filter $(call lnot,$1),$(call lnot,$2)),y,n)
ne = $(if $(filter $(call lnot,$1),$(call lnot,$2)),n,y)

> +isempty = $(call eq,$1,)
> +notempty = $(call ne,$1,)

These are wrong assuming we want 'n' eq '' semantics,
and we can directly implement them with $if anyway:

isempty = $(if $1,n,y)
notempty = $(if $1,y,n)

Below is the test makefile I used to check these:
# insert function definitions here

# change this to whichever fn you are looking at

.PHONY: all

    @echo "$(testfn) yes,yes = $(call $(testfn),$(yes),$(yes))"
    @echo "$(testfn) yes,no = $(call $(testfn),$(yes),$(no))"
    @echo "$(testfn) no,yes = $(call $(testfn),$(no),$(yes))"
    @echo "$(testfn) no,no = $(call $(testfn),$(no),$(no))"
    @echo "$(testfn) yes,empty = $(call $(testfn),$(yes),$(empty))"
    @echo "$(testfn) empty,yes = $(call $(testfn),$(empty),$(yes))"
    @echo "$(testfn) empty,no = $(call $(testfn),$(empty),$(no))"
    @echo "$(testfn) no,empty = $(call $(testfn),$(no),$(empty))"
    @echo "$(testfn) empty,empty = $(call $(testfn),$(empty),$(empty))"
    @echo "isempty($(empty)) = $(call isempty,$(empty))"
    @echo "isempty($(full)) = $(call isempty,$(full))"
    @echo "notempty($(empty)) = $(call notempty,$(empty))"
    @echo "notempty($(full)) = $(call notempty,$(full))"

Since I've effectively rewritten all your patch here :-)
I'll put together a patch plus your patches 2 and 3
from this series since I think they make a nice standalone

-- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]