On Thu, 3 Apr 2025 20:15:00 GMT, Mikael Vidstedt <mik...@openjdk.org> wrote:
>> We have been sloppy in our use of `printf` in make code. Most of the time, >> we should really use `echo` instead. If we do need to use `printf`, we >> should never inline make or shell variables into the formatting string, >> since they may contain `%` which will be interpreted as formatting. Instead, >> we should always use `%s` and pass the variable as an argument to `printf`. >> >> I've checked the entire code base for usages of $PRINTF, and converted most >> of them to $ECHO, and made sure the remaining ones are correct. I also >> discovered some additional ugly stuff in relation to this, which I fixed. > > make/Docs.gmk line 267: > >> 265: $$(call LogInfo, Creating overview.html for $1) >> 266: $$(call MakeDir, $$(@D)) >> 267: $$(PRINTF) "%s" '$$($1_OVERVIEW_TEXT)' > $$@ > > For my education, why isn't this just `$$(ECHO) '$$($1_OVERVIEW_TEXT)' > $$@` > ? That would add an extra \n at the end. Maybe it does not matter so much, but I did not want to change anything. > make/Init.gmk line 163: > >> 161: $(PARALLEL_TARGETS) $(COMPARE_BUILD_MAKE) >> $(BUILD_LOG_PIPE) || \ >> 162: ( exitcode=$$? && \ >> 163: $(ECHO) "" $(BUILD_LOG_PIPE_SIMPLE) && \ > > Just for my understanding, is the empty string needed, or "just" a stylistic > choice? Yeah, it is a stylistic choice to indicate that the line was intentionally left empty. > make/autoconf/help.m4 line 295: > >> 293: $ECHO "* HS debug level: $HOTSPOT_DEBUG_LEVEL" >> 294: $ECHO "* JVM variants: $JVM_VARIANTS" >> 295: $ECHO "* JVM features: " > > I believe this will now add a newline where there was none earlier, so the > actual features will show up on a new line? Good catch; I'll fix that. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027743799 PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027744477 PR Review Comment: https://git.openjdk.org/jdk/pull/24415#discussion_r2027745056
