Re: [Xen-devel] [PATCH OSSTEST] sg-run-job: Only consider $OSSTEST_SIMULATE > 0

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("[PATCH OSSTEST] sg-run-job: Only consider 
$OSSTEST_SIMULATE > 0"):
> This matches the semantics implemented in cri-args-hostlists and is
> compatible with the recent change to the standalone wrapper
> ("standalone: Add --dry-run option for run-job.") which sets it to 0
> or 1.

Oops!  This patch should be inserted before that one, then.

>      set xprefix {}
> -    if {[info exists env(OSSTEST_SIMULATE)]} { set xprefix echo }
> +    if {[info exists env(OSSTEST_SIMULATE)] &&
> +     [expr $env(OSSTEST_SIMULATE) > 0]} {

Not enclosing the expression with { } is very poor style, although
here it happens to work.

Also: there is another occurrence of the same pattern, in
JobDB-Executive.tcl:

  proc db-execute-debug {stmt} {
      if {[info exists env(OSSTEST_TCL_JOBDB_DEBUG)]} {

I suggest something like this:

  diff --git a/tcl/osstestlib.tcl b/tcl/osstestlib.tcl
  index a0413c4..61a6a09 100644
  --- a/tcl/osstestlib.tcl
  +++ b/tcl/osstestlib.tcl
  @@ -74,3 +74,9 @@ proc lshift {listvar} {
       set list [lrange $list 1 end]
       return $head
   }
  +
  +proc var-or-default {varname {default {}}} {
  +    upvar 1 $varname var
  +    if {[info exists var]} { return $var }
  +    return $default
  +}

(Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>)

and then we can write

  if {[var-or-default env(OSSTEST_SIMULATE) 0]} { ...

And if you set OSSTEST_SIMULATE to "wombat" you get a backtrace, with
a message from the Tcl interpreter saying:

   expected boolean value but got "wombat"

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel