On 6/24/19 11:55 AM, Andrey Shinkevich wrote: >>> +++ b/tests/qemu-iotests/common.rc >>> @@ -17,6 +17,8 @@ >>> # along with this program. If not, see <http://www.gnu.org/licenses/>. >>> # >>> >>> +readonly VALGRIND_SUPPRESS_ERRORS=./valgrind.supp >> >> Why readonly? >> >> I think it should be defined near and in same manner as VALGRIND_LOGFILE, >> with use of TEST_DIR >> > > The new file 'valgrind.supp' is intended to be a part of the QEMU > project. So, it will be located in the test directory tests/qemu-iotests. > The variable TEST_DIR may change the working directory. In that case, > moving the project file will be a hassle.
My personal thoughts are that no serious POSIX or bash shell script ever uses readonly (it offers the illusion of security but cannot actually back it up, and in reality ends up causing more bugs than it prevents when your script breaks because you tried to modify a readonly variable). I've only ever dealt with one project that tried to use readonly in earnest (the 'cygport' script for building Cygwin packages) and it got in the way more than it saved me from bugs. Declaring that VALGRIND_SUPPRESS_ERRORS is initialized hard-coded to ./ instead of relative to ${TEST_DIR} is orthogonal to whether you declare that the variable VALGRIND_SUPPRESS_ERRORS can no longer be further modified by the rest of the script. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature