Bug 385939

Summary: Optionally exit on the first error
Product: [Developer tools] valgrind Reporter: Fauchet Gauthier <gauthier.fauchet>
Component: generalAssignee: Ivo Raisr <ivosh>
Status: RESOLVED FIXED    
Severity: wishlist CC: gauthier.fauchet, ivosh
Priority: NOR    
Version First Reported In: 3.13 SVN   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: patch for --exit-on-fisrt-error implementation
patch for --exit-on-fisrt-error implementation version 2

Description Fauchet Gauthier 2017-10-19 08:24:04 UTC
Created attachment 108453 [details]
patch for --exit-on-fisrt-error implementation

It would be great if Valgrind could exit on first error. Useful if you are running regression tests or have some other automated test machinery.


I have implented --exit-on-first-error=<yes|no>.

Doc:
If this option is enabled, Valgrind exits on first error. A nonzero exit value must be defined using --error-exitcode option.

In attachment,  git diff svn/VALGRIND_3_13_0 >   patch_exit_on_first_error.txt
Comment 1 Ivo Raisr 2017-10-24 07:35:19 UTC
I will have a look.
Comment 2 Ivo Raisr 2017-10-24 08:24:18 UTC
Thank you for the patch and kudos for documenting the new option in the manual.


From the high level perspective, I have the following comments and questions:
* It is not clear what happens in the XML mode. Is the XML correctly finalized with --exit-on-first-error=yes?
* Valgrind does not print anything after it encounters the first error with --exit-on-first-error=yes, even with '-v'. This could be potentially confusing to the users. However they get what they wanted, anyway.
* There is no test case present in the patch. Please supply a test case which exercises --exit-on-first-error=yes in text and XML mode. The test case will belong under none/tests.


From the lower level perspective, I have the following comments:
* Source code block indentation is 3 spaces (yes, 3 spaces). Please check all occurrences.
* New option is listed in usage_NORETURN() but none/tests/cmdline{1,2].stdout.exp do not reflect it. Please run the regression tests.
* Please do not introduce trailing whitespace.
Comment 3 Fauchet Gauthier 2017-11-02 13:42:32 UTC
Hello,
thank you for your comments.

I have made some corrections:
* The XML is now correctly finalized with --exit-on-first-error=yes.
* A user message is displayed before exit
* 2 new test cases :  badaddrvalue_exit_on_first_error and badaddrvalue_exit_on_first_error_with_xml in memcheck/tests.
* indentation is now 3 spaces.
* non/tests/cmdline*stdout.exp are updated
* no trailing whitespace.
Comment 4 Fauchet Gauthier 2017-11-02 13:43:39 UTC
Created attachment 108679 [details]
patch for --exit-on-fisrt-error implementation  version 2
Comment 5 Ivo Raisr 2017-11-04 13:33:44 UTC
Pushed as commit c46053cc386be5757420abbd4b8a8cc6219ed33f.

https://sourceware.org/git/?p=valgrind.git;a=commit;h=c46053cc386be5757420abbd4b8a8cc6219ed33f

Thank you for the reworked patch.