diff mbox

posix: Add SIGSEGV on the trap list for globtest.sh

Message ID 11eed88b-109b-bfb3-4e5c-be086aa87689@linaro.org
State New
Headers show

Commit Message

Adhemerval Zanella Netto April 12, 2017, 12:35 p.m. UTC 
On 11/04/2017 18:12, Florian Weimer wrote:
> On 04/11/2017 09:59 PM, Adhemerval Zanella wrote:
>>
>>
>> On 11/04/2017 15:22, Florian Weimer wrote:
>>> On 04/11/2017 08:09 PM, Adhemerval Zanella wrote:
>>>> -trap 'chmod 777 $testdir/noread; rm -fr $testdir $testout' 1 2 3 15
>>>> +trap 'chmod 777 $testdir/noread; rm -fr $testdir $testout' HUP INT QUIT SEGV TERM
>>>
>>> Does the shell really receive the signal?  Why not use “0”?
>>
>> It does not off course since it is not the shell that is actually generating
>> the segfault.
> 
> Well, the shell uses globbing, too, so who knows. :)
> 
>> I will rewrite the patch.
> 
> Thanks.  Using 0 is probably the correct choice, then.
> 
> Florian

Ok, here is a version 2.

--

This patch prevents lingering files for SIGSEGV failures by adding
a cleanup handler on trap handler.  Checked on x86_64-linux-gnu.

	* posix/globtest.sh: Add cleanup for the signal to act on a trap.

---
 ChangeLog         |  4 ++++
 posix/globtest.sh | 10 +++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

index f9cc80b..a764aef 100755

Comments

Florian Weimer April 12, 2017, 1:19 p.m. UTC | #1 
On 04/12/2017 02:35 PM, Adhemerval Zanella wrote:
> Ok, here is a version 2.
> 
> --
> 
> This patch prevents lingering files for SIGSEGV failures by adding
> a cleanup handler on trap handler.  Checked on x86_64-linux-gnu.
> 
> 	* posix/globtest.sh: Add cleanup for the signal to act on a trap.

Looks reasonable.

> +cleanup()

Not sure what the preferred style is here, “cleanup () {” or yours.

GNU style for shell scripts seems to be 4 space indentation.

Thanks,
Florian
Adhemerval Zanella Netto April 12, 2017, 1:49 p.m. UTC | #2 
On 12/04/2017 10:19, Florian Weimer wrote:
> On 04/12/2017 02:35 PM, Adhemerval Zanella wrote:
>> Ok, here is a version 2.
>>
>> -- 
>>
>> This patch prevents lingering files for SIGSEGV failures by adding
>> a cleanup handler on trap handler.  Checked on x86_64-linux-gnu.
>>
>>     * posix/globtest.sh: Add cleanup for the signal to act on a trap.
> 
> Looks reasonable.
> 
>> +cleanup()
> 
> Not sure what the preferred style is here, “cleanup () {” or yours.
> 
> GNU style for shell scripts seems to be 4 space indentation.
> 
> Thanks,
> Florian

GLIBC usage seems to tend for 'function () {', although there both usage
across the scripts.  I will change it and use 4 space indentation.
Florian Weimer April 12, 2017, 2:09 p.m. UTC | #3 
On 04/12/2017 03:19 PM, Florian Weimer wrote:

> GNU style for shell scripts seems to be 4 space indentation.

I may have been wrong about that.  It is what Emacs uses, but GNU is 
inconsistent.  Two spaces are common as well.

Thanks,
Florian
diff mbox

Patch

--- a/posix/globtest.sh
+++ b/posix/globtest.sh
@@ -47,7 +47,13 @@  testout=${common_objpfx}posix/globtest-out
 rm -rf $testdir $testout
 mkdir $testdir
 
-trap 'chmod 777 $testdir/noread; rm -fr $testdir $testout' 1 2 3 15
+cleanup()
+{
+  chmod 777 $testdir/noread
+  rm -fr $testdir $testout
+}
+
+trap cleanup 0 HUP INT QUIT TERM
 
 echo 1 > $testdir/file1
 echo 2 > $testdir/file2
@@ -811,8 +817,6 @@  if test $failed -ne 0; then
 fi
 
 if test $result -eq 0; then
-    chmod 777 $testdir/noread
-    rm -fr $testdir $testout
     echo "All OK." > $logfile
 fi