From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115718 invoked by alias); 25 Feb 2018 22:41:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 115707 invoked by uid 89); 25 Feb 2018 22:41:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*M:4c11 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Feb 2018 22:41:42 +0000 Received: from [10.0.0.11] (192-222-251-162.qc.cable.ebox.net [192.222.251.162]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DEA2D1E584; Sun, 25 Feb 2018 17:41:40 -0500 (EST) Subject: Re: [PATCH] Improve gcore shell quoting and portability To: Georg Sauthoff , gdb-patches@sourceware.org References: <20180225204601.19068-1-mail@georg.so> From: Simon Marchi Message-ID: <6c710a6d-5d12-c2eb-4c11-9c28196f3866@simark.ca> Date: Sun, 25 Feb 2018 22:41:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180225204601.19068-1-mail@georg.so> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-02/txt/msg00368.txt.bz2 Hi Georg, Thanks for the patch. I checked the resulting script (with your patch applied) with https://www.shellcheck.net, and it doesn't give any important warning/errors anymore (only suggests replacing `...` with $(...)). On 2018-02-25 03:46 PM, Georg Sauthoff wrote: > The gcore shell script (gdb/gcore.in) doesn't quote its variables > enough. > > For example, trying to write a core file with - say - a space > ungraciously fails like this: > > $ gcore -o 'foo bar' 6270 > /usr/bin/gcore: line 92: [: foo: binary operator expected > gcore: failed to create foo bar.6270 > > Similarly, one can inject meta characters like * (by accident) > that may yield unexpected results, e.g. as in: > > $ gcore -o foobar '*' > > This change fixes these issues in several places. > > Aso, since the script uses array syntax, the patch changes the > the shell in the first line from `/bin/sh` to /bin/bash`. > > POSIX doesn't specify the array syntax for shell, thus, the > script doesn't work on systems where /bin/sh is linked to - say - > dash. > > Since the source gcore.in already is processed by a pre-processor > one could even auto-detect the path to bash and thus dynamically > generate the first line. For systems where bash isn't available > via /bin/bash. But I think this would be overkill and /bin/bash > is good enough as most systems probably have it. The script will not necessarily run on the same machine than the one where it is preprocessed, so it's not possible to determine the path to bash in advance. I think the usual way to do it is #!/usr/bin/env bash > gdb/ChangeLog: > > PR gdb/22888 > * gcore.in: quote variables and switch interpreter to bash Missing period at the end. > --- > gdb/gcore.in | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/gdb/gcore.in b/gdb/gcore.in > index b7f57cd3..816c5629 100644 > --- a/gdb/gcore.in > +++ b/gdb/gcore.in > @@ -1,4 +1,4 @@ > -#!/bin/sh > +#!/bin/bash > > # Copyright (C) 2003-2018 Free Software Foundation, Inc. > > @@ -28,9 +28,9 @@ name=core > dump_all_cmds=() > > while getopts :ao: opt; do > - case $opt in > + case "$opt" in > a) > - case $OSTYPE in > + case "$OSTYPE" in > linux*) > dump_all_cmds=("-ex" "set use-coredump-filter off") > dump_all_cmds+=("-ex" "set dump-excluded-mappings on") > @@ -93,16 +93,16 @@ fi > rc=0 > > # Loop through pids > -for pid in $* > +for pid in "$@" > do > # ` # available but not accessible as GDB would get stopped on SIGTTIN. > - $binary_path/@GDB_TRANSFORM_NAME@ + "$binary_path"/@GDB_TRANSFORM_NAME@