Hi, On Tue, Aug 27, 2013 at 06:44:09PM -0400, Daniel Kahn Gillmor wrote: > On 08/27/2013 05:40 PM, Andreas Mohr wrote: > > (witness --skip getting overwritten by any subsequent --new / --overwrite > > in this while loop) > > > > > > The root cause of my confusion case is that -n -o -s parameters are > > wholly redundant: instead, they are supposed to be a *single* *mode* switch, > > to cleanly reflect exactly how it's implemented within-script > > (WRITE_MODE=xxx), > > since this seems appropriate here. > > > > These redundant options should thus be deprecated in favour of e.g. a > > combined > > -m (--mode) switch taking rewrite/overwrite/skip arguments, I'd think > > (and probably add a deprecation warning for a while, > > whenever the old switches happen to get used). > > If i understand correctly, checking for redundant options and > terminating with a non-zero error code and a warning would have avoided > your problem without breaking compatibility with existing scripts that > use only one of the options. > > I'd welcome a patch that implements such a check. I'd rather not add a > new command-line option (e.g. --mode) if we can avoid it.
Yeah, it's probably better after all to preserve userspace behaviour, to reduce confusion. > Thanks for pointing this out! N.p., I've got some more debirf hints in the pipe ;) Preliminary untested (just got rid of my environment here) patch (any comments? improvements?): commit 860e666815264c179141289c6e7d59a8f753a252 Author: Andreas Mohr <[email protected]> Date: Wed Aug 28 10:00:59 2013 +0200 Protect against the user passing multiple different debirf write mode options. Since there are multiple redundant -n/-o/-s switches rather than a single mode switch, we need to guard against the user passing multiple incompatible write mode requests. diff --git a/src/debirf b/src/debirf index d1a54cc..d7b1f17 100755 --- a/src/debirf +++ b/src/debirf @@ -349,14 +349,17 @@ make() { ;; -n|--new) WRITE_MODE=rewrite + write_mode_rewrite_given=1 shift 1 ;; -o|--overwrite) WRITE_MODE=overwrite + write_mode_overwrite_given=1 shift 1 ;; -s|--skip) WRITE_MODE=skip + write_mode_skip_given=1 shift 1 ;; -r|--root-build) @@ -393,6 +396,18 @@ make() { ;; esac done + # Protect against misuse of redundant write mode implementation + # (multiple -n/-o/-s switches), see #721094. + # Do this by adding extra helper flags above + # that are to be increment-evaluated here, + # to avoid disallowing e.g. a duplicate -s -s case. + WRITE_MODE_ARGCOUNT=0 + [ -n "${write_mode_rewrite_given}" ] && WRITE_MODE_ARGCOUNT=$((WRITE_MODE_ARGCOUNT+1)) + [ -n "${write_mode_overwrite_given}" ] && WRITE_MODE_ARGCOUNT=$((WRITE_MODE_ARGCOUNT+1)) + [ -n "${write_mode_skip_given}" ] && WRITE_MODE_ARGCOUNT=$((WRITE_MODE_ARGCOUNT+1)) + if [ ${WRITE_MODE_ARGCOUNT} -gt 1 ]; then + failure "Multiple different write mode switches (-n/-o/-s) given - please resolve this command line conflict." + fi if [ $(id -u) = '0' ] ; then cat <<EOF Thanks, Andreas Mohr -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
