Re: [Qemu-devel] [PATCH] rules.mak: Force CFLAGS for all objects in DSO

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/05/2015 16:23, Fam Zheng wrote:
>>> > > +             $(eval $($v:%.mo=%$(DSOSUF)) $($v) $(foreach o,$($v),$($o-objs)) .PHONY: CFLAGS += -fPIC -DBUILD_DSO)
>                                 ^               ^                ^                   ^
>                                 |               |                |                   |
>                                 |               |                |                   `- In case all others are empty.
>                                 |               |                |
>                                 |               |                `- Expansion of all %.mo-objs so it's a %.o list.
>                                 |               |                   For $v=block-obj-m, this will contain curl.o, iscsi.o, ...
>                                 |               |
>                                 |               `- %.mo list
>                                 |                  For $v=block-obj-m, this will contain curl.mo, iscsi.mo, ...
>                                 |
>                                 `- %.so list
>                                    For $v=block-obj-m, this will contain curl.so, iscsi.so, .

Great. :)  You should have used Unicode drawing characters to be
consistent with rules.mak!

But hopefully the ASCII art is not needed at all.  Do we need all of those?

- foo.mo is a prerequisite of foo.so.  You cannot use foo.so (this is
the bug you are fixing) but foo.so doesn't use CFLAGS in its rule, and
you should be able to remove it.

- can you use foo.mo without hitting the original bug?  If so, could you
simply use:

-%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
+%.mo: CFLAGS += -fPIC -DBUILD_DSO

(and if so move the rule down, above "%.mo:")?  If you cannot use
foo.mo, foo.mo also doesn't use CFLAGS in its rules, and you should be
able to remove foo.mo too.

- so the other possibility is to just use $(foreach o,$($v),$($o-objs)).
 In this case there's already a convenient $(foreach) just above, and
you can just add another $(eval) inside it.

- and if that is true, you do not need .PHONY either because you have
$(foreach ... $(eval)) instead of $(eval $(foreach)).

Maybe I'm wrong, in which case the patch is ok. :)

Paolo