This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [ping3][PATCH 01/19 v2] Add new macro IN_MODULE to identify module in which source is built
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Fri, 7 Nov 2014 15:22:10 -0800 (PST)
- Subject: Re: [ping3][PATCH 01/19 v2] Add new macro IN_MODULE to identify module in which source is built
- Authentication-results: sourceware.org; auth=none
- References: <1408618663-2281-1-git-send-email-siddhesh at redhat dot com> <1408618663-2281-2-git-send-email-siddhesh at redhat dot com> <20140822182247 dot E0FCE2C398C at topped-with-meat dot com> <20140827171916 dot GE8020 at spoyarek dot pnq dot redhat dot com> <20140917123835 dot GU6586 at spoyarek dot pnq dot redhat dot com> <20141001081613 dot GG2217 at spoyarek dot pnq dot redhat dot com>
> Patches that follow will remove uses of IS_IN_* variables with the
s/remove/replace/
> Verified that there are no relevant source changes. One source change
> that will crop up repeatedly is that of nscd_stat, since it uses the
> build timestamp as a constant in its logic.
s/source/binary/
> +in-module = $(strip $(foreach lib,$(libof-$(basename $(@F))) $(libof-$(<F)) \
> + $(libof-$(@F)),-DIN_MODULE=MODULE_$(lib)))
I think you can make this:
in-module = $(firstword $(libof-$(basename $(@F))) \
$(libof-$(<F)) \
$(libof-$(@F))
libc)
There should never be more than one nonempty libof-* expansion unless it's
multiple that are the same. Also don't repeat the fixed parts
(i.e. -DIN_MODULE=MODULE_).
> - $(CPPFLAGS-$(suffix $@)) \
> + $(CPPFLAGS-$(suffix $@)) $(module-def) \
Here just use -DIN_MODULE=MODULE_$(in-module) directly.
> diff --git a/elf/rtld-Rules b/elf/rtld-Rules
> index 0a5d6af..4d78d90 100644
> --- a/elf/rtld-Rules
> +++ b/elf/rtld-Rules
> @@ -138,6 +138,11 @@ ifdef rtld-depfiles
> -include $(rtld-depfiles)
> endif
>
> +# Set libof-* for each routine.
> +cpp-srcs-left := $(subst .os,,$(rtld-modules))
Use $(rtld-modules:%.os=%) (or patsubst if you prefer).
Plain subst will eat .os out of the middle of a name, which is wrong.
> --- /dev/null
> +++ b/include/libc-modules.h
Even though it's just for a brief window of the revision history, please
put a comment in this file saying it should/will be generated.
> +#include "libc-modules.h"
Use <>. Add a short comment saying that is defines the MODULE_* macros.
> -CPPFLAGS-locale-programs = -DLOCALE_PATH='$(localepath)' \
> +CPPFLAGS-locale_programs = -DLOCALE_PATH='$(localepath)' \
Rather than changing this name, I think the libc-modules.h generation
should just turn all nonidentifier characters into _.
OK with those changes.
Thanks,
Roland