Re: [Xen-devel] [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU sandboxing

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

George Dunlap writes ("Re: [PATCH v2 6/6] RFC: tools/dm_restrict: Enable QEMU 
sandboxing"):
> On 09/25/2018 12:02 PM, Ian Jackson wrote:
> > If you don't say set -e then you need to wrap every everything in your
> > entire script with an error check.

(I have deleted the whole comparative programming languages
discussion.)

> > For example, you write
> > 
> >>>> dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid 
> >>>> 2>/dev/null)
> >>>> if [[ -z "$dmpid" ]] ; then
> >>>>     echo "xenstore-read failed"
> >>>>     exit 1
> >>>> fi
...
> > but with set -e you can write only
> > 
> >     dmpid=$(xenstore-read /local/domain/$domid/image/device-model-pid)
> > 
> > and the subsequent if is not needed.
> 
> I think you misunderstood what I meant.  The original code looked
> something like this:

Yes.  You agreed to the removal of the 2>/dev/null.  My point here is
that you if you use set -e you can get rid of the entire error
clause.

> In the current code, printing the error message is obviously better than
> throwing it away.  But I still feel better checking the result and
> giving a sensible follow-up error message than having a failure silently
> exit, because I prefer to explicitly think about what happens when
> things fail (and remind the people reading the code to do so as well).

This approach is not very usual in shell scripting (at least, in
circles whose behaviour should be emulated)[1], and it leads to bugs.
Your script as originally presented is riddled with unchecked calls to
subprograms.

If you absolutely want to handle a particular error case specifically
then IMO it is much better to make a specific exception to that
particular case, by using either a set +e / set -e pair, or a
construct like if or ||.

> > The expression
> >   ${input#*  }
...
> That's an idea.  I'm not sure I like it much better than using a regexp
> (now that the runes have been written), but I'll think about it.

Fair enough.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel