On 13.07.2021 16:58, Anthony PERARD wrote: > On Tue, Jul 13, 2021 at 09:51:45AM +0200, Jan Beulich wrote: >> On 12.07.2021 18:32, Anthony PERARD wrote: >>> On Wed, Jul 07, 2021 at 05:48:57PM +0200, Jan Beulich wrote: >>>> On 01.07.2021 16:09, Anthony PERARD wrote: >>>>> --- a/xen/common/Makefile >>>>> +++ b/xen/common/Makefile >>>>> @@ -80,8 +80,12 @@ config.gz: $(CONF_FILE) >>>>> >>>>> config_data.o: config.gz >>>>> >>>>> -config_data.S: $(BASEDIR)/tools/binfile >>>>> - $(SHELL) $(BASEDIR)/tools/binfile $@ config.gz xen_config_data >>>>> +quiet_cmd_binfile = BINFILE $@ >>>>> +cmd_binfile = $(SHELL) $< $@ config.gz xen_config_data >>>> >>>> This is an abuse of $< which I consider overly confusing: >>>> $(BASEDIR)/tools/binfile is not the input file to the rule. Instead >>>> the script generates an assembly file "out of thin air", with not >>>> input files at all. The rule and ... >>>> >>>>> +config_data.S: $(BASEDIR)/tools/binfile FORCE >>>> >>>> ... dependency shouldn't give a different impression. What would >>>> be nice (without having checked how difficult this might be) would >>>> be if quiet_cmd_binfile and cmd_binfile could move to xen/Rules.mk >>>> and merely be used from here (and the other location, where the >>>> same concern obviously applies). >>> >>> I've though of having cmd_binfile in Rules.mk, but it's made more >>> complicated by having a "-i" flag used in flask/. >>> >>> So one things I've writen was: >>> >>> config_data.S: $(BASEDIR)/tools/binfile FORCE >>> $(call if_changed,binfile,,config.gz xen_config_data) >>> flask-policy.S: $(BASEDIR)/tools/binfile FORCE >>> $(call if_changed,binfile,-i,policy.bin xsm_flask_init_policy) >>> >>> with: >>> cmd_binfile = $(SHELL) $(BASEDIR)/tools/binfile $(2) $@ $(3) >>> >>> I thought this would be confusing, so I avoid it. >> >> Indeed that's why I did write "without having checked how difficult >> this might be", because I definitely didn't want to suggest such >> anomalies to get introduced. It's unhelpful that options have to >> come first. >> >>> But maybe with the lists of flags at the end, it would be better: >>> $(call if_changed,binfile,policy.bin xsm_flask_init_policy,-i) >> >> Yes, this is a little better imo, but still not pretty. >> >>> Still want to move cmd_binfile to Rules.mk? >> >> I'd still like it to be moved, but without resulting in a rule >> that's not consistent with others. Maybe we should have a >> BINFILE_FLAGS variable (paralleling e.g. CFLAGS)? > > Sound good. > > cmd_binfile = $(SHELL) $(BASEDIR)/tools/binfile $(BINFILE_FLAGS) $@ $(2) > > flask-policy.S: BINFILE_FLAGS := -i > flask-policy.S: $(BASEDIR)/tools/binfile FORCE > $(call if_changed,binfile,policy.bin xsm_flask_init_policy) > > Would the above be OK? > Otherwise, maybe you'll prefer the following: > > flask-policy.S: BINFILE_FLAGS = -i $@ policy.bin xsm_flask_init_policy > flask-policy.S: $(BASEDIR)/tools/binfile FORCE > $(call if_changed,binfile)
I think I like the former better than the latter, but I could live with either. Jan
