| Message ID | 9850917e60104f97809fac670d9ba68ea5130ac0.1490091134.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 21, 2017 at 12:14:31PM +0200, Jani Nikula wrote: > Fix shellcheck SC2115: Use "${var:?}" to ensure this never expands to /. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> I'm not sold on these because if those are ever unset, dim will fail all over the place. I'm leaning towards shutting this one up. -Daniel > --- > dim | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/dim b/dim > index 0fe125335a67..8bbff67b3f80 100755 > --- a/dim > +++ b/dim > @@ -461,18 +461,26 @@ function rr_cache_dir > > function update_rerere_cache > { > + local rerere > + > + rerere=$(rr_cache_dir) > + > cd $DIM_PREFIX/drm-rerere/ > git pull > - mkdir $(rr_cache_dir) &> /dev/null || true > - cp rr-cache/* $(rr_cache_dir) -r > + mkdir ${rerere:?} &> /dev/null || true > + cp rr-cache/* ${rerere:?} -r > cd - > /dev/null > } > > function dim_revert_rerere > { > + local rerere > + > + rerere=$(rr_cache_dir) > + > cd $DIM_PREFIX/drm-rerere/ > git revert $1 > - rm $(rr_cache_dir)/* -Rf > + rm -Rf ${rerere:?}/* > } > > dim_alias_rebuild_nightly=rebuild-tip > @@ -974,7 +982,7 @@ function dim_remove_branch > cd $DIM_PREFIX/$DIM_DRM_INTEL > > if [[ -d $DIM_PREFIX/$branch ]] ; then > - rm -R $DIM_PREFIX/$branch > + rm -R ${DIM_PREFIX:?}/$branch > git worktree prune &> /dev/null || true > fi > > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 21 Mar 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Mar 21, 2017 at 12:14:31PM +0200, Jani Nikula wrote: >> Fix shellcheck SC2115: Use "${var:?}" to ensure this never expands to /. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > I'm not sold on these because if those are ever unset, dim will fail all > over the place. Yup, like at the top where we specifically ensure $DIM_PREFIX is not empty! > I'm leaning towards shutting this one up. I know, I was divided about this one as well, but erred on the safe side because 'rm -Rf /*' is such a huge failure mode. I could lean either way. BR, Jani. > -Daniel > >> --- >> dim | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/dim b/dim >> index 0fe125335a67..8bbff67b3f80 100755 >> --- a/dim >> +++ b/dim >> @@ -461,18 +461,26 @@ function rr_cache_dir >> >> function update_rerere_cache >> { >> + local rerere >> + >> + rerere=$(rr_cache_dir) >> + >> cd $DIM_PREFIX/drm-rerere/ >> git pull >> - mkdir $(rr_cache_dir) &> /dev/null || true >> - cp rr-cache/* $(rr_cache_dir) -r >> + mkdir ${rerere:?} &> /dev/null || true >> + cp rr-cache/* ${rerere:?} -r >> cd - > /dev/null >> } >> >> function dim_revert_rerere >> { >> + local rerere >> + >> + rerere=$(rr_cache_dir) >> + >> cd $DIM_PREFIX/drm-rerere/ >> git revert $1 >> - rm $(rr_cache_dir)/* -Rf >> + rm -Rf ${rerere:?}/* >> } >> >> dim_alias_rebuild_nightly=rebuild-tip >> @@ -974,7 +982,7 @@ function dim_remove_branch >> cd $DIM_PREFIX/$DIM_DRM_INTEL >> >> if [[ -d $DIM_PREFIX/$branch ]] ; then >> - rm -R $DIM_PREFIX/$branch >> + rm -R ${DIM_PREFIX:?}/$branch >> git worktree prune &> /dev/null || true >> fi >> >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Mar 21, 2017 at 01:15:59PM +0200, Jani Nikula wrote: > On Tue, 21 Mar 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, Mar 21, 2017 at 12:14:31PM +0200, Jani Nikula wrote: > >> Fix shellcheck SC2115: Use "${var:?}" to ensure this never expands to /. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > I'm not sold on these because if those are ever unset, dim will fail all > > over the place. > > Yup, like at the top where we specifically ensure $DIM_PREFIX is not > empty! > > > I'm leaning towards shutting this one up. > > I know, I was divided about this one as well, but erred on the safe side > because 'rm -Rf /*' is such a huge failure mode. > > I could lean either way. Even for this failure mode it feels like noise ... I guess bash scripting just is noise, and we'll live with it? Saying, Ḯ'm also ok with you pushing this. We might eventually have a command where the input is user-supplied and if absent could result in tears. I just fixed that broken default handling for dim retip :-) -Daniel
On Tue, Mar 21, 2017 at 01:50:48PM +0100, Daniel Vetter wrote: > On Tue, Mar 21, 2017 at 01:15:59PM +0200, Jani Nikula wrote: > > On Tue, 21 Mar 2017, Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Tue, Mar 21, 2017 at 12:14:31PM +0200, Jani Nikula wrote: > > >> Fix shellcheck SC2115: Use "${var:?}" to ensure this never expands to /. > > >> > > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > > > > > I'm not sold on these because if those are ever unset, dim will fail all > > > over the place. > > > > Yup, like at the top where we specifically ensure $DIM_PREFIX is not > > empty! > > > > > I'm leaning towards shutting this one up. > > > > I know, I was divided about this one as well, but erred on the safe side > > because 'rm -Rf /*' is such a huge failure mode. > > > > I could lean either way. > > Even for this failure mode it feels like noise ... I guess bash scripting > just is noise, and we'll live with it? > > Saying, Ḯ'm also ok with you pushing this. We might eventually have a > command where the input is user-supplied and if absent could result in > tears. I just fixed that broken default handling for dim retip :-) rm -rf / woes can be remedied by always passing "--preserve-root" to rm (I believe that this is the default already though). Kind regards, David
On Wed, 22 Mar 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > rm -rf / woes can be remedied by always passing "--preserve-root" to rm > (I believe that this is the default already though). That doesn't help with 'rm -rf /*' though. BR, Jani.
On Wed, Mar 22, 2017 at 12:53:24PM +0200, Jani Nikula wrote: > On Wed, 22 Mar 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > > rm -rf / woes can be remedied by always passing "--preserve-root" to rm > > (I believe that this is the default already though). > > That doesn't help with 'rm -rf /*' though. Hmmm, but how would you end up with that if it does /$foo and $foo="" or unset? Kind regards, David
On Wed, 22 Mar 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: > On Wed, Mar 22, 2017 at 12:53:24PM +0200, Jani Nikula wrote: >> On Wed, 22 Mar 2017, David Weinehall <david.weinehall@linux.intel.com> wrote: >> > rm -rf / woes can be remedied by always passing "--preserve-root" to rm >> > (I believe that this is the default already though). >> >> That doesn't help with 'rm -rf /*' though. > > Hmmm, but how would you end up with that if it does /$foo and $foo="" or > unset? There's a 'rm -rf $foo/*'
diff --git a/dim b/dim index 0fe125335a67..8bbff67b3f80 100755 --- a/dim +++ b/dim @@ -461,18 +461,26 @@ function rr_cache_dir function update_rerere_cache { + local rerere + + rerere=$(rr_cache_dir) + cd $DIM_PREFIX/drm-rerere/ git pull - mkdir $(rr_cache_dir) &> /dev/null || true - cp rr-cache/* $(rr_cache_dir) -r + mkdir ${rerere:?} &> /dev/null || true + cp rr-cache/* ${rerere:?} -r cd - > /dev/null } function dim_revert_rerere { + local rerere + + rerere=$(rr_cache_dir) + cd $DIM_PREFIX/drm-rerere/ git revert $1 - rm $(rr_cache_dir)/* -Rf + rm -Rf ${rerere:?}/* } dim_alias_rebuild_nightly=rebuild-tip @@ -974,7 +982,7 @@ function dim_remove_branch cd $DIM_PREFIX/$DIM_DRM_INTEL if [[ -d $DIM_PREFIX/$branch ]] ; then - rm -R $DIM_PREFIX/$branch + rm -R ${DIM_PREFIX:?}/$branch git worktree prune &> /dev/null || true fi
Fix shellcheck SC2115: Use "${var:?}" to ensure this never expands to /. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- dim | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)