Re: Add sigaltstack() prototype - Round 2

[Joel Sherrill <joel dot sherrill at oarcorp dot com>]

On 8/15/2014 9:32 AM, Corinna Vinschen wrote:
> On Aug 15 09:20, Joel Sherrill wrote:
>> On 8/15/2014 8:46 AM, Corinna Vinschen wrote:
>>> On Aug 15 08:26, Joel Sherrill wrote:
>>>> 2014-08-15 Joel Sherrill <...>
>>>>
>>>>     * libc/include/sys/signal.h: Add sigaltstack() support.
>>> Shouldn't this patch define the available flags, too?
>> You are correct. It should. Obviously the code we were compiling didn't
>> use them. :)
>>
>> Attached is v3. Values follow those in glibc. Same ChangeLog.
> Uhm, I'm sorry, but...
No reason to be sorry. It is a review. :)
>> Index: newlib/libc/include/sys/signal.h
>> ===================================================================
>> RCS file: /cvs/src/src/newlib/libc/include/sys/signal.h,v
>> retrieving revision 1.22
>> diff -u -r1.22 signal.h
>> --- newlib/libc/include/sys/signal.h	26 Oct 2012 09:23:46 -0000	1.22
>> +++ newlib/libc/include/sys/signal.h	15 Aug 2014 14:20:10 -0000
>> @@ -104,6 +104,23 @@
>>  #define sa_sigaction  _signal_handlers._sigaction
>>  #endif
>>  
>> +#if __BSD_VISIBLE || __XSI_VISIBLE || __POSIX_VISIBLE >= 200112
>> +/*
>> + * Possible values for ss_flags in stack_t below.
>> + */
>> +#define SS_ONSTACK 0x1
>> +#define SS_ONSTACK 0x2
> ...that's SS_DISABLE 0x2, right?

Right.

I need to slow down on a Friday morning. I am ready to get out in the
pretty weather. :(

And FWIW the use of tabs vs spaces in defining constants in this file
is very inconsistent.
> POSIX also defines SIGSTKSZ and MINSIGSTKSZ. 
And checking the signal.h Open Group page, there is also SA_ONSTACK. The
SA_xxx constants are in OS specific sections so I added this only to the
RTEMS section with the same guards.
>  I'm wondering if we
> shouldn't add default values here if the variables aren't already
> defined.  Or would that rather be something for the target specific
> config?
They should be target specific but we could default them if not defined
in <sys/features.h>. <sys/features.h> is already included so that is already
a dependency.

My CentOS 6.x defines them in <asm/signal.h> to this:

#define MINSIGSTKSZ     2048
#define SIGSTKSZ        8192

Those seem reasonable enough as defaults the target can override.

Attached is another revision. Don't fret over catching my mistakes. It
is part of the process.

>
> Corinna
>

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel.sherrill@OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
Support Available                (256) 722-9985

Attachment: sigaltstack-v5.diff
Description: Text document