Re: [Qemu-block] [Qemu-devel] [PATCH v2 7/7] iotests: new file to suppress Valgrind errors

[Andrey Shinkevich]

On 24/06/2019 20:09, Eric Blake wrote:
> 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.
> 

Thank you Eric. I am flexible on that subject. If the path is relative 
to ${TEST_DIR}, should the file valgrind.supp be copied from ./ to the 
${TEST_DIR} by the script itself?

Andrey
-- 
With the best regards,
Andrey Shinkevich