On 10/26/2018 03:06 PM, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 5/5] RFC: test/depriv: Add a tool to check
> process-level depriv"):
>> Add a tool to check whether the various process-level deprivileging
>> operations have actually taken place on the process.
> ...
>> NB that a number of other requested changes (such as using `set -e`,
>> changing the output, &c) have not been made, while I consider whether
>> to leave this as a stand-alone script, or whether to merge osstest's
>> fd checker functionality into it (perhaps changing the language to perl
>> at the same time).
>
> OK. But, unfortunately, it is very hard to review a shell script that
> is written without `set -e'. Generally a big focus of my usual review
> style is error handling.
>
> Also, I suggested some refactoring. Seeing the script as it is makes
> it more obvious that a systematic approach to printing FAILED
> etc. would be a good idea.
FYI I do agree with all of those suggestions (both `set -e` and having
functions to handle failure in a consistent way); but I didn't want to
fix everything up in bash only to have to write it over again in perl
(should I decide to do so). I wasn't expecting a review for this patch
yet; I only included it for completeness. I guess I figured "I didn't
make all the changes you wanted" would make that obvious, but next time
I'll be more explicit when I don't expect a patch to be reviewed. :-)
Thanks,
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
