All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Michal Marek <mmarek@suse.cz>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-kbuild <linux-kbuild@vger.kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	"x86@kernel.org" <x86@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] kbuild: escape single backslashes in macro make-cmd
Date: Thu, 7 Aug 2014 19:08:00 +0200	[thread overview]
Message-ID: <20140807170800.GA1542@ravnborg.org> (raw)
In-Reply-To: <20140807160739.GA29726@sepie.suse.cz>

> 
> The problem is that the bash echo does not interpred _any_ \-sequences
> without -e. So the nicely escaped \\ stays a \\ and we get spurious
> rebuilds with bash instead. It previously worked with bash, because the
> backslash escaping was completely broken. You not only moved it up in
> the chain, but also fixed it by removing one superflous level of
> escaping.
> 
> I'm now testing this, i.e. dropping the backslash escaping completely
> and doing a printf '%s\n' instead. Can you try if it works for you?
> 
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 122f95c..8a9a4e1 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -215,11 +215,13 @@ else
>  arg-check = $(if $(strip $(cmd_$@)),,1)
>  endif
>  
> -# >'< substitution is for echo to work,
> -# >$< substitution to preserve $ when reloading .cmd file
> -# note: when using inline perl scripts [perl -e '...$$t=1;...']
> -# in $(cmd_xxx) double $$ your perl vars
> -make-cmd = $(subst \\,\\\\,$(subst \#,\\\#,$(subst $$,$$$$,$(call escsq,$(cmd_$(1))))))
> +# Replace >$< with >$$< to preserve $ when reloading the .cmd file
> +# (needed for make)
> +# Replace >#< with >\#< to avoid starting a comment in the .cmd file
> +# (needed for make)
> +# Replace >'< with >'\''< to be able to enclose the whole string in '...'
> +# (needed for the shell)
> +make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
This is simpler and remotely readble - nice!
As a bonus you added more descriptive comments

Looks good.
You may add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

  parent reply	other threads:[~2014-08-07 17:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-26 16:35 [PATCH] kbuild: escape single backslashes in macro make-cmd Konstantin Khlebnikov
2014-08-06 11:45 ` Michal Marek
2014-08-06 12:19   ` Konstantin Khlebnikov
2014-08-06 15:01     ` Michal Marek
2014-08-07 16:07       ` Michal Marek
2014-08-07 16:54         ` Konstantin Khlebnikov
2014-08-07 17:08         ` Sam Ravnborg [this message]
2014-08-07 19:56           ` [PATCH] kbuild: Fix handling of backslashes in *.cmd files Michal Marek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140807170800.GA1542@ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=akpm@linux-foundation.org \
    --cc=koct9i@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=s.hauer@pengutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.