On Sun, Feb 25, 2024 at 09:03:04AM -0800, Darrick J. Wong wrote: > On Sun, Feb 25, 2024 at 08:51:28AM -0800, Eric Biggers wrote: > > On Sun, Feb 25, 2024 at 11:16:16PM +0800, Zorro Lang wrote: > > > On Wed, Feb 21, 2024 at 03:09:51PM +0100, David Sterba wrote: > > > > Hi, > > > > > > > > reading [1] and how late it was found that effectively a "rm -rf /" can > > > > happen makes me worried about what I can expect from fstests after git > > > > pull. Many people contribute and the number for custom _cleanup() > > > > functions with unquoted 'rm' commands is just asking for more problems. > > > > > > > > [1] https://lore.kernel.org/all/20240205060016.7fgiyafbnrvf5chj@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > > > > > > > > Unquoted arguments in shell scripts is IMO a big anti-pattern, > > > > unfortunately present everywhere in xfstests since the beginning. > > > > Rewriting all scripts would be quite a lot of work, could you at least > > > > provide safe versions of the cleanup helpers? > > > > > > Hi David, > > > > > > Thanks for taking care about it :) > > > > > > > > > > > For example: > > > > > > > > _rm_tmp() { > > > > rm -rf -- $tmp > > > > > > It's "$tmp.*" > > > > > > May I ask what problem does the "--" hope to avoid? If the "$tmp" is empty, > > > "rm -rf" and "rm -rf --"" looks like both doing nothing. So what kind > > > of situation does the "--" hope to fix? > > > > > > The root problem in above [1] is about "${FOO}*". If someone does "rm -rf ${FOO}*" > > > in its custom _cleanup_xxxxx function, then it's dangerous if "$FOO" is empty. > > > > > > I thought some ways to avoid that: > > > 1) Try to avoid doing rm -rf ${FOO}*, if not necessary. > > > 2) Must checks [ -n "$FOO" ] before doing any rm -rf ${FOO}* > > > 3) Someone's custom _cleanup_xxxxx better to be called before default _cleanup > > > does "cd /". > > > 4) Think about bringing in someone "Static program analysis" tool about bash > > > script, but I don't know if there're someone good, feel free to give me > > > suggestions. > > > > "--" prevents the following arguments from being interpreted as options if they > > begin with "-". That's a good practice, but it doesn't help with ${FOO} being > > empty. To cause the script to exit if ${FOO} is empty, it can be written as > > ${FOO:?}. Alternatively, 'set -u' can be used. > > I said that four days ago. Did nobody receive that reply? > > https://lore.kernel.org/fstests/20240225165128.GA1128@sol.localdomain/T/#m0efd851c5a1fb0dbe418f4aff818d20f4355638b Sorry Darrick, I've noticed your reply. About the "set -u", it helps to avoid empty variables, but the question is when should we set it. If we set it at the beginning of each case, won't it affect too much? Some cases might don't mind empty variables, but this setting might cause some cases fail. The programmers might want to decide if they accept empty variables by themselves? Thanks, Zorro > > --D > > > - Eric > > >
