| Message ID | 83e8b90e1d2cc5ff5d2443f2486c2d786a4997ce.1376600922.git.yann.morin.1998@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 15, 2013 at 11:17 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > From: Clement Chauplannaz <chauplac@gmail.com> > > Script `config' relies on extensions of `GNU sed', and is thus not > working on all Unixes: > - in-place edition of files (-i), which can be replaced with > a temporary file; > - extended-regexps (-r), which can be split into basic regexps; > - single-line calls to `a' command, while some implementations > require a leading newline before the parameter. > > Rewrite calls to `sed' to comply with POSIX interface, and move them > to helper functions. > > Signed-off-by: Clement Chauplannaz <chauplac@gmail.com> > Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> This patch totally breaks my usage of the --set-str argument to "config". Reverting this patch solves the problem. Reproduce: make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- KBUILD_OUTPUT=build-nomadik nhk8815_defconfig scripts/config --file build-nomadik/.config --set-str CMDLINE "root=/dev/ram0 console=ttyAMA1,115200n8 earlyprintk" sed: -e uttryck #1, tecken 44: flaggan okänd för "s" sed: -e uttryck #1, tecken 54: flaggan okänd för "s" Swedish messages meaning "unknown flag for "s"" After reverting the patch these messages no longer appear. At failure my config file is scratched :-O Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2013/9/12 Linus Walleij <linus.walleij@linaro.org>: > On Thu, Aug 15, 2013 at 11:17 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > >> From: Clement Chauplannaz <chauplac@gmail.com> >> >> Script `config' relies on extensions of `GNU sed', and is thus not >> working on all Unixes: >> - in-place edition of files (-i), which can be replaced with >> a temporary file; >> - extended-regexps (-r), which can be split into basic regexps; >> - single-line calls to `a' command, while some implementations >> require a leading newline before the parameter. >> >> Rewrite calls to `sed' to comply with POSIX interface, and move them >> to helper functions. >> >> Signed-off-by: Clement Chauplannaz <chauplac@gmail.com> >> Tested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> >> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> >> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr> > > This patch totally breaks my usage of the --set-str > argument to "config". > > Reverting this patch solves the problem. > > Reproduce: > make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- > KBUILD_OUTPUT=build-nomadik nhk8815_defconfig > scripts/config --file build-nomadik/.config --set-str CMDLINE > "root=/dev/ram0 console=ttyAMA1,115200n8 earlyprintk" > sed: -e uttryck #1, tecken 44: flaggan okänd för "s" > sed: -e uttryck #1, tecken 54: flaggan okänd för "s" > > Swedish messages meaning "unknown flag for "s"" > > After reverting the patch these messages no longer appear. > > At failure my config file is scratched :-O > > Yours, > Linus Walleij Hello Linus, Thank you for this report. I was able to reproduce this bug and fix it. My previous commit changed the separator between sed's substitute command and its parameters, from ':' to '/'. The latter conflicted with the slashes found in the value of variable CMDLINE, as provided in your email. I'm sending a patch right now to revert to previous behavior. Best regards, Clement Chauplannaz -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 13, 2013 at 10:38 AM, Clément Chauplannaz <chauplac@gmail.com> wrote: > Thank you for this report. I was able to reproduce this bug and fix it. Thanks! Tested and works fine. > My previous commit changed the separator between sed's substitute > command and its parameters, from ':' to '/'. The latter conflicted > with the slashes found in the value of variable CMDLINE, as provided > in your email. Hm it could actually be useful to be able to have colons in a CMDLINE, I wonder if we can think about some better separator ... oh well that is another issue, all old scripts work now anyway. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sep 13, 2013, at 11:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Fri, Sep 13, 2013 at 10:38 AM, Clément Chauplannaz > <chauplac@gmail.com> wrote: > >> Thank you for this report. I was able to reproduce this bug and fix it. > > Thanks! Tested and works fine. Glad to read the patch solves your issue. Thanks for the quick feedback! > >> My previous commit changed the separator between sed's substitute >> command and its parameters, from ':' to '/'. The latter conflicted >> with the slashes found in the value of variable CMDLINE, as provided >> in your email. > > Hm it could actually be useful to be able to have colons in a CMDLINE, > I wonder if we can think about some better separator ... oh well that > is another issue, all old scripts work now anyway. Indeed config script may not work with all possible string values. My first concern for now was to fallback to previous interface. We may look into hardening the script later on. Best regards, Clement Chauplannaz-- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne 13.9.2013 11:54, Clément Chauplannaz napsal(a): > On Sep 13, 2013, at 11:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> On Fri, Sep 13, 2013 at 10:38 AM, Clément Chauplannaz >> <chauplac@gmail.com> wrote: >> >>> Thank you for this report. I was able to reproduce this bug and fix it. >> >> Thanks! Tested and works fine. > Glad to read the patch solves your issue. Thanks for the quick feedback! >> >>> My previous commit changed the separator between sed's substitute >>> command and its parameters, from ':' to '/'. The latter conflicted >>> with the slashes found in the value of variable CMDLINE, as provided >>> in your email. >> >> Hm it could actually be useful to be able to have colons in a CMDLINE, >> I wonder if we can think about some better separator ... oh well that >> is another issue, all old scripts work now anyway. > Indeed config script may not work with all possible string values. > My > first concern for now was to fallback to previous interface. We may look > into hardening the script later on. Right. I will merge the patch because it reverts a regression. But feel free to submit another patch that escapes the colons in $after. The script already uses #!/bin/bash, so a "${after//:/\:}" should work. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/config b/scripts/config index 567120a..2283be2 100755 --- a/scripts/config +++ b/scripts/config @@ -62,15 +62,52 @@ checkarg() { fi } +txt_append() { + local anchor="$1" + local insert="$2" + local infile="$3" + local tmpfile="$infile.swp" + + # sed append cmd: 'a\' + newline + text + newline + cmd="$(printf "a\\%b$insert" "\n")" + + sed -e "/$anchor/$cmd" "$infile" >"$tmpfile" + # replace original file with the edited one + mv "$tmpfile" "$infile" +} + +txt_subst() { + local before="$1" + local after="$2" + local infile="$3" + local tmpfile="$infile.swp" + + sed -e "s/$before/$after/" "$infile" >"$tmpfile" + # replace original file with the edited one + mv "$tmpfile" "$infile" +} + +txt_delete() { + local text="$1" + local infile="$2" + local tmpfile="$infile.swp" + + sed -e "/$text/d" "$infile" >"$tmpfile" + # replace original file with the edited one + mv "$tmpfile" "$infile" +} + set_var() { local name=$1 new=$2 before=$3 name_re="^($name=|# $name is not set)" before_re="^($before=|# $before is not set)" if test -n "$before" && grep -Eq "$before_re" "$FN"; then - sed -ri "/$before_re/a $new" "$FN" + txt_append "^$before=" "$new" "$FN" + txt_append "^# $before is not set" "$new" "$FN" elif grep -Eq "$name_re" "$FN"; then - sed -ri "s:$name_re.*:$new:" "$FN" + txt_subst "^$name=.*" "$new" "$FN" + txt_subst "^# $name is not set" "$new" "$FN" else echo "$new" >>"$FN" fi @@ -79,7 +116,8 @@ set_var() { undef_var() { local name=$1 - sed -ri "/^($name=|# $name is not set)/d" "$FN" + txt_delete "^$name=" "$FN" + txt_delete "^# $name is not set" "$FN" } if [ "$1" = "--file" ]; then