On Sun, Feb 25, 2024 at 09:45:27AM -0800, Eric Biggers wrote: > 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 Thanks Eric, I know "--" can do that, just didn't understand how it helps the empty variable problem. So looks like it doesn't. > > > 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 > > > > You didn't mention the :? option, and I thought that would be worth mentioning. Actually: [ -n "$FOO" ] && rm -rf ${FOO}* or rm -rf ${FOO:?}* Both of them are good to me, depends on what's expected. The 1st one ignore empty $FOO and keep running, the 2nd one breaks case running if $FOO is empty. But they all need the programer to realize that his variable might be dangerous if it's empty, and write as that. So it still depends on the programer or the reviewers to notice that. Thanks, Zorro > > Of course ideally -u would be used everywhere, as you said. > > - Eric >
