refactor HAS_SIGPROCMASK in Perl_despatch_signals

p5pRT commented 10 years ago

Migrated from rt.perl.org#121100 (status was 'open')

Searchable as RT121100$

p5pRT commented 10 years ago

From @bulk88

Created by @bulk88

I can't refactor this since this code doesn't exist on Win32. Hence this ticket. Read the // comments.

https://github.com/Perl/perl5/blob/fb29cc72fe9ef39100123477fffae4fad34866b7/mg.c#L1461

void
Perl_despatch_signals(pTHX)
{
dVAR;
int sig;
PL_sig_pending = 0;
for (sig = 1; sig < SIG_SIZE; sig++) {
if (PL_psig_pend[sig]) {
dSAVE_ERRNO;
#ifdef HAS_SIGPROCMASK
/* From sigaction(2) (FreeBSD man page):
* | Signal routines normally execute with the signal that
* | caused their invocation blocked, but other signals may
* | yet occur.
* Emulation of this behavior (from within Perl) is enabled
* using sigprocmask
*/
int was_blocked;
sigset_t newset, oldset;

sigemptyset(&newset);
sigaddset(&newset, sig);
sigprocmask(SIG_BLOCK, &newset, &oldset);
was_blocked = sigismember(&oldset, sig);
if (!was_blocked) {
//instead of SVs, why not Newx and SAVEFREEPV? or instead of SAVEFREEPV
put a Safefree into unblock_sigmask()?
SV* save_sv = newSVpvn((char *)(&newset), sizeof(sigset_t)); // dont
assign here
ENTER;
// save_sv = newSVpvn((char *)(&newset), // put me here
for locality.
SAVEFREESV(save_sv);
SAVEDESTRUCTOR_X(unblock_sigmask, SvPV_nolen(save_sv)); //SvPVX since
no magic and gurenteed PV, and switch from SAVEDESTRUCTOR_X to
SAVEDESTRUCTOR since my_perl isn't used in unblock_sigmask
}
--------------------------------------------------------------------
#if defined HAS_SIGPROCMASK
static void
unblock_sigmask(pTHX_ void* newset) //remove pTHX_
{
sigprocmask(SIG_UNBLOCK, (sigset_t*)newset, NULL);
}
#endif

Perl Info

``` Flags: category=core severity=low Site configuration information for perl 5.19.7: Configured by Owner at Thu Nov 28 02:32:44 2013. Summary of my perl5 (revision 5 version 19 subversion 7) configuration: Derived from: 8f47723e28b75530b743941cdd8b07f849ec48e2 Ancestor: 1061065f7a09399eefb50e9a035502621722bcc0 Platform: osname=MSWin32, osvers=5.1, archname=MSWin32-x86-multi-thread uname='' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=undef, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cl', ccflags ='-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -G7 -GL -DWIN32 -D_CONSOLE -DNO_STRICT -DPERL_TEXTMODE_SCRIPTS -DPERL_HASH_FUNC_ONE_AT_A_TIME -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO -D_USE_32BIT_TIME_T', optimize='-O1 -MD -Zi -DNDEBUG -G7 -GL', cppflags='-DWIN32' ccversion='13.10.6030', gccversion='', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=8 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' libpth="C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\lib" libs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib perllibs=oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib comctl32.lib msvcrt.lib libc=msvcrt.lib, so=dll, useshrplib=true, libperl=perl519.lib gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -ltcg -libpath:"c:\perl519\lib\CORE" -machine:x86' Locally applied patches: uncommitted-changes 8f47723e28b75530b743941cdd8b07f849ec48e2 @INC for perl 5.19.7: C:/perl519/site/lib C:/perl519/lib . Environment for perl 5.19.7: HOME (unset) LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=C:\perl519\bin;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE;C:\Program Files\Microsoft Visual Studio .NET 2003\VC7\BIN;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools;C:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools\bin\prerelease;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\system32\wbem; PERL_BADLANG (unset) SHELL (unset) ```

p5pRT commented 10 years ago

From @tux

On Mon\, 27 Jan 2014 22:45:05 -0800\, bulk88 (via RT) \perlbug\-followup@perl\.org wrote:

I can't refactor this since this code doesn't exist on Win32. Hence this ticket. Read the // comments.

Triggers twitching eye: // comment are a no-go: they will break many C89 compilers

-- H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/ using perl5.00307 .. 5.19 porting perl5 on HP-UX\, AIX\, and openSUSE http://mirrors.develooper.com/hpux/ http://www.test-smoke.org/ http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/

p5pRT commented 10 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 10 years ago

From @bulk88

The // comments were so that nobody gets really lazy and commits my comments unchanged as "todo"\, without reading the comments or ticket body.

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 10 years ago

From perl5-porters@perl.org

H.Merijn Brand wrote:

Triggers twitching eye: // comment are a no-go: they will break many C89 compilers

That's the point. These are to-do comments.

p5pRT commented 10 years ago

From @Leont

On Tue\, Jan 28\, 2014 at 7:45 AM\, bulk88 \perlbug\-followup@perl\.org wrote:

//instead of SVs\, why not Newx and SAVEFREEPV? or instead of SAVEFREEPV put a Safefree into unblock_sigmask()?

Ehm\, I don't know. Probably I was so used to using SAVEFREESV that SAVEFREEPV didn't occur to me.

            SV\* save\_sv = newSVpvn$\(char \*$$&newset$\,
sizeof(sigset_t)); // dont assign here ENTER; // save_sv = newSVpvn((char *)(&newset)\, // put me here for locality.

*Shrugs*

            SAVEFREESV$save\_sv$;
            SAVEDESTRUCTOR\_X$unblock\_sigmask\, SvPV\_nolen\(save\_sv$\);
//SvPVX since no magic and gurenteed PV\, and switch from SAVEDESTRUCTOR_X to SAVEDESTRUCTOR since my_perl isn't used in unblock_sigmask

Is there really any advantage of SAVEDESTRUCTOR over SAVEDESTRUCTOR_X? It's more of a habit of mine to avoid the former\, though in this case it doesn't really matter.

Leon

toddr commented 4 years ago

@Leont @bulk88 Is there a PR we can make from this or is this a dead discussion?

Perl / perl5