bugGNU Octave - Bugs: bug #39089, Incomplete list of OS signals in...

 
 

bug #39089: Incomplete list of OS signals in the builtin SIG function

Submitted by:  Dan Sebald <sebald>
Submitted on:  Mon 27 May 2013 08:43:45 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 03 Jun 2013 08:55:53 PM UTC, comment #13:

I just sent a sample routine to gnulib-list that will return all strings (for which the existing str2sig can be used to get the signal number). Let's wait a few days to see what they think.

Dan Sebald <sebald>
Sun 02 Jun 2013 07:30:15 PM UTC, comment #12:

The gnulib sig2str.h file has been updated for C++ use, but I don't know how long that takes to filter through the gnulib-hg repository.

Attached is a changeset that will use sig2str() to get every signal used. The list is then sorted alphabetically. EXIT is excluded, SIGUNUSED is not checked for. There are no duplicate definitions, so behavior is slightly different than the current behavior. If we want the behavior to be exactly the same, after the loop using sig2str() we can add a few #ifdef's to check for the known duplicates.

The gnulib list is considering the issue of non-uniquely defined signals. So, we may want to wait on gnulib to see if there is a change, or simply go with the attached changeset and make small mods at a later date if gnulib changes.

(file #28225)

Dan Sebald <sebald>
Sat 01 Jun 2013 05:05:25 PM UTC, comment #11:

Ah yes, the C/C++ variation. OK, I'll send an email and changeset to libgnu.

One question is how we should handle duplicates. Right now Octave is including duplicates in the list, e.g.,

But sig2str() can't do that because the input is a number and the routine doesn't return multiple definitions for that number, just picks the first occurrence.

is what results from scanning gnulibs sig2str(). Now, I sort of like what Octave is currently doing, and it is quite easy for us to add in the few cases where there are duplicates. (There is a str2sig routine that allows us to check if "CLD", "IOT", etc. are defined.) Furthermore, we can do so in a way that will avoid any problems in which GNULIB arranges things internally by, say, checking for both "CLD" and "CHLD" (m.assign() applied twice on an assignment should be no problem). On the other hand, this is kind of bad programming practice in the sense we are duplicating something that GNULIB's sig2str() should be doing--they do keep track of both CLD and CHLD but sig2str can only return one of them.

So, I'm wondering if we should make an argument to GNULIB to consider changing sig2str() so that it somehow returns all the signal definitions for a given number. They would need some internal string buffer array or something and return a pointer to it. It just feels to me that the sig2str() routine isn't complete the way it is.

Also, GNULIB is returning "EXIT" for the number zero in all cases:

Should I exclude the number 0? I think so.

Also, there is this SIGUNUSED in my signum.h file. GNULIB doesn't check for a SIGUNUSED. Should we leave SIGUNUSED out of Octave's list? Or keep it in to reflect exactly what the user's system is defining?

Dan Sebald <sebald>
Fri 31 May 2013 02:02:23 PM UTC, comment #10:

Also, you do not need to add libgnu.la to the list of libraries in the libinterp/Makefile.am file.

John W. Eaton <jwe>
Project Administrator
Fri 31 May 2013 01:59:37 PM UTC, comment #9:

The gnulib sig2str.h header doesn't put the functions it declares inside an 'extern "C"' block.

The gnulib maintainers have been responsive to requests for adding 'extern "C"' blocks to headers in the past.

John W. Eaton <jwe>
Project Administrator
Fri 31 May 2013 05:29:19 AM UTC, comment #8:

Also, if this works, perhaps we should add an option "installed" or "handled" or "listen" to SIG to indicate which signals Octave has installed a handler for. E.g.,

SIG ("system") is same as current SIG

SIG ("handled") is subset of SIG("system")

Dan Sebald <sebald>
Fri 31 May 2013 05:10:22 AM UTC, comment #7:

Attached is a changeset that (as far as I know) removes siglist.h and siglist.c and constructs the SIG list by inquiring the sig2str() routine in gnulib. This sort of obviates the previous patch, but should be nice because then all possible SIGxxxx are accounted for. If the sys_siglist is required again at some point, we can go to gnulib for that.

Unfortunately, the changeset compiles but does not link. I couldn't figure out what the issue is. The object code from the gnulib sig2str module is present somewhere but isn't showing up in

I tried adding the gnulib.la library to the libinterp Makefile.am but that didn't help.

./bootstrap is required to build.

(file #28213)

Dan Sebald <sebald>
Thu 30 May 2013 09:34:59 PM UTC, comment #6:

I guess that strsignal isn't the complete list, just the BSD strings:

http://infohost.nmt.edu/~eweiss/222_book/222_book/0201433079/ch10lev1sec21.html
http://www.unix.com/man-page/all/3c/sig2str/
http://www.unix.com/man-page/all/3c/strsignal/

Anyway, I don't see where sys_siglist[] is used anywhere in the code. So perhaps we could just remove siglist.h and siglist.c from Octave?

Also, we might be able to simplify the SIG routine so that make_sig_struct doesn't require definitions. In the gnulib sig2str.c is this structure:

which is the exact same contents as make_sig_struct. Unfortunately, this a local definition inaccessible to the project code. However, we could loop through all signal numbers and use this routine to inquire for a name:

i.e.,

The consequence of that is that the structure will now be listed in numerical order, not alphabetical. But we could use Octave's sort routine internally to construct a new structure that is in alphabetical order.

I'll do that this evening if you think it is a good idea.

Dan Sebald <sebald>
Thu 30 May 2013 08:50:32 PM UTC, comment #5:

Ohh, here's the full hunk of code associated with sys_siglist in that header file:

This sys_siglist now looks like legacy support for BSD (and no wonder the limited number of definitions). The comment makes it sound like we should be using "strsignal" now, which has the full complement of definitions and extensions.

Dan Sebald <sebald>
Thu 30 May 2013 08:45:50 PM UTC, comment #4:

I see now that gnulib has restructured things slightly, perhaps because it now appears there is a "sys_siglist" in the standard kernel code(?). gnulib's siglist.h is not actually a header file, per se. It is used in strsignal.c similar to C macro code. The header file defines things like:

which is a macro expanded inside strsignal.c as:

Via gnulib I see no header file declaration for "sys_siglist".

I do see such a declaration in my systems header file tree though:

[sebald@ include]$ grep sys_siglist * -r
signal.h:extern __const char *__const _sys_siglist[_NSIG];
signal.h:extern __const char *__const sys_siglist[_NSIG];

So maybe things have progressed to the point where this sys_siglist doesn't even need to be constructed anymore and gnulib will cover the scenario whereby some operating system doesn't yet do so.

Dan Sebald <sebald>
Thu 30 May 2013 08:11:24 PM UTC, comment #3:

Ha, there looks to be a bug in the list software when underscore follows "verbatim" delimiter. Here's the original message removing the "verbatim" delimiter to see if the whole thing goes through this time:

Thank you... This gnulib siglist.h/siglist.c looks like it might be a good idea. I'm guessing that signals.h (bits/signum.h) is constructed as part of the kernel compilation process (or maybe it is library level). siglist.h/siglist.c is meant to reflect that. Here's a siglist.c from far back

http://www.opensource.apple.com/source/bash/bash-30/bash/siglist.c

that is similar to Octave's. Octave's siglist.h has a comment referencing Emacs. So perhaps Octave signal interpretation predate's gnulib's code.

The gnulib github has a recent copyright suggesting it is fairly well maintained. There's a gnulib file strsignal.c that seems to be the equivalent of Octave's siglist.c but gets the description from somewhere that isn't obvious (INTUSE ?) and I'm not sure why it is adding an underscore _sys_siglist which is typically done when referencing C variables inside assembly language code.

The siglist.h in gnulib doesn't have the following from Octave's siglist.h:

The only two of these that are defined on my system are SIGCLD (#17 which is the same as SIGCHLD) and SIGPOLL (#29 which is the same as SIGIO). So my system should be covered by gnulib's siglist.h.

There is also a sig2str.c in gnulib which does contain all the definitions including those I listed above, along with a short comment categorizing them as

There are a half dozen other groups of extensions as well. So the signal interpretation does seem to be covered fairly well in gnulib.

Dan Sebald <sebald>
Thu 30 May 2013 08:01:13 PM UTC, comment #2:

Thank you... This gnulib siglist.h/siglist.c looks like it might be a good idea. I'm guessing that signals.h (bits/signum.h) is constructed as part of the kernel compilation process (or maybe it is library level). siglist.h/siglist.c is meant to reflect that. Here's a siglist.c from far back

http://www.opensource.apple.com/source/bash/bash-30/bash/siglist.c

that is similar to Octave's. Octave's siglist.h has reference to Emacs. So perhaps Octave signal interpretation predate's gnulib's code.

Dan Sebald <sebald>
Thu 30 May 2013 06:23:57 PM UTC, comment #1:

I aplied your patch:

http://hg.savannah.gnu.org/hgweb/octave/rev/736dca8371ee

There is also a siglist.h file in gnulib. Maybe we should be using that instead of our own.

John W. Eaton <jwe>
Project Administrator
Mon 27 May 2013 08:43:45 PM UTC, original submission:

The files siglist.c and sighandlers.cc are incomplete as far as a list of signals SIGxxxxxxx that could appear in the Unix/Linux header file signals.h (or bits/signum.h). For example, typing the following commands:

will produce a list of signals that doesn't quite match what is inside the signum.h header file on my system. These are missing:

So the attached changeset adds a few of these signals so that the SIG function (and the sorting above) produces the same list that appears in my signum.h. However, this is not a complete list for everyone's system as there are more entries listed here:

http://dfrench.hypermart.net/fancyIndex/Tools/signal2.html

for example, and still more obscure ones floating about some of which already appear in siglist.c and/or sighandlers.cc.

Dan Sebald <sebald>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #28225:  octave-sighandlers-2013jun02.patch added by sebald (13KiB - application/octet-stream)
file #28213:  octave-sighandlers-2013may30.patch added by sebald (13KiB - application/octet-stream)
file #28190:  octave-signal_missing-2013may27.patch added by sebald (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by sebald (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 02 Jun 2013 07:30:15 PM UTCsebaldAttached File-=>Added octave-sighandlers-2013jun02.patch, #28225
    Fri 31 May 2013 05:10:22 AM UTCsebaldAttached File-=>Added octave-sighandlers-2013may30.patch, #28213
    Thu 30 May 2013 06:23:57 PM UTCjweStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Mon 27 May 2013 08:43:45 PM UTCsebaldAttached File-=>Added octave-signal_missing-2013may27.patch, #28190

    Back to the top


    Powered by Savane 3.1-cleanup1