{"thread":{"id":"36798","subject":"[PATCH v2 0/2] replace signal() with sigaction()","startedAt":"2014-05-30T20:58:15Z","lastAt":"2014-05-31T14:45:16Z","messageCount":6,"participants":["Jeremiah Mahler","Andreas Schwab","Chris Packham"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"243053","messageId":"cover.1401482787.git.jmmahler@gmail.com","threadId":"36798","inReplyTo":null,"subject":"[PATCH v2 0/2] replace signal() with sigaction()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-30T20:58:15Z","receivedAt":"2014-05-30T20:58:15Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"This is the second revision to the patch set to replace signal(2) with\nsigaction(2) [1].\n\nAs Johannes pointed out [2], replacing signal with sigaction would break\nMinGW compatibility.  The first patch in this series addresses this\nproblem by expanding the faux sigaction function in compat/mingw.c to\nsupport signals other than just SIGALRM.  Details are in the patch\ndescription.\n\nThe second patch is a proof of concept.  It converts signal to sigaction\nin a case where signal SIGCHILD was used.  Previously this would have\nfailed with MinGW since the faux sigaction function only supported\nSIGALRM.  Now it works as expected.\n\nI have tested these changes under Linux and under Windows 7 using Msysgit.\n\n[1]: http://marc.info/?l=git&m=140125769223552&w=2\n[2]: http://marc.info/?l=git&m=140126288325213&w=2\n\nJeremiah Mahler (2):\n  compat/mingw.c: expand MinGW support for sigaction\n  connect.c: replace signal() with sigaction()\n\n compat/mingw.c | 9 +++++----\n connect.c      | 5 ++++-\n 2 files changed, 9 insertions(+), 5 deletions(-)\n\n-- \n2.0.0.2.g1d11d5d\n"},{"id":"243054","messageId":"d5fc7d72b2d51d1b90fb7f238eff120cb4c6d0e7.1401482787.git.jmmahler@gmail.com","threadId":"36798","inReplyTo":"cover.1401482787.git.jmmahler@gmail.com","subject":"[PATCH v2 1/2] compat/mingw.c: expand MinGW support for sigaction","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-30T20:58:16Z","receivedAt":"2014-05-30T20:58:16Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Due to portability issues across UNIX versions sigaction(2) should be used\ninstead of signal(2).\n\nFrom the signal(2) man page:\n\n  The behavior of signal() varies across UNIX versions, and has also var‐\n  ied historically across different versions of Linux.   Avoid  its  use:\n  use sigaction(2) instead.\n\nUnfortunately MinGW under Windows has limited support for signal and no\nsupport for sigaction.  And this prevents sigaction from being used across\nthe entire Git project.\n\nIn compat/mingw.c there is a faux sigaction function but it only supports\nSIGALARM.  Hence the need for continuing to use signal() in other cases.\n\nThis patch expands the faux sigaction function so that it calls signal in\ncases other than SIGALRM.  Now sigaction can be used across the entire Git\nproject and MinGW will still work with signal as it did before.\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n---\n compat/mingw.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex e9892f8..e504cef 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -1651,14 +1651,15 @@ int setitimer(int type, struct itimerval *in, struct itimerval *out)\n \n int sigaction(int sig, struct sigaction *in, struct sigaction *out)\n {\n-\tif (sig != SIGALRM)\n-\t\treturn errno = EINVAL,\n-\t\t\terror(\"sigaction only implemented for SIGALRM\");\n \tif (out != NULL)\n \t\treturn errno = EINVAL,\n \t\t\terror(\"sigaction: param 3 != NULL not implemented\");\n \n-\ttimer_fn = in->sa_handler;\n+\tif (sig == SIGALRM)\n+\t\ttimer_fn = in->sa_handler;\n+\telse\n+\t\tsignal(sig, in->sa_handler);\n+\n \treturn 0;\n }\n \n-- \n2.0.0.2.g1d11d5d\n"},{"id":"243055","messageId":"1d11d5da7ebc3a6e0d4ab64802b601526c19113c.1401482787.git.jmmahler@gmail.com","threadId":"36798","inReplyTo":"cover.1401482787.git.jmmahler@gmail.com","subject":"[PATCH v2 2/2] connect.c: replace signal() with sigaction()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-30T20:58:17Z","receivedAt":"2014-05-30T20:58:17Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"From signal(2) man page:\n\n  The behavior of signal() varies across UNIX versions, and has also var‐\n  ied historically across different versions of Linux.   Avoid  its  use:\n  use sigaction(2) instead.\n\nReplaced signal() with sigaction() in connect.c\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n---\n connect.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/connect.c b/connect.c\nindex a983d06..b2a33c9 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -665,11 +665,14 @@ struct child_process *git_connect(int fd[2], const char *url,\n \tenum protocol protocol;\n \tconst char **arg;\n \tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct sigaction sa;\n \n \t/* Without this we cannot rely on waitpid() to tell\n \t * what happened to our children.\n \t */\n-\tsignal(SIGCHLD, SIG_DFL);\n+\tmemset(&sa, 0, sizeof(sa));\n+\tsa.sa_handler = SIG_DFL;\n+\tsigaction(SIGCHLD, &sa, 0);\n \n \tprotocol = parse_connect_url(url, &hostandport, &path);\n \tif (flags & CONNECT_DIAG_URL) {\n-- \n2.0.0.2.g1d11d5d\n"},{"id":"243061","messageId":"87sinr5457.fsf@igel.home","threadId":"36798","inReplyTo":"1d11d5da7ebc3a6e0d4ab64802b601526c19113c.1401482787.git.jmmahler@gmail.com","subject":"Re: [PATCH v2 2/2] connect.c: replace signal() with sigaction()","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-30T21:48:52Z","receivedAt":"2014-05-30T21:48:52Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jeremiah Mahler <jmmahler@gmail.com> writes:\n\n> From signal(2) man page:\n>\n>   The behavior of signal() varies across UNIX versions, and has also var‐\n>   ied historically across different versions of Linux.   Avoid  its  use:\n>   use sigaction(2) instead.\n\nI don't think this matters for SIG_DFL or SIG_IGN.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"243072","messageId":"5389B16B.6070004@gmail.com","threadId":"36798","inReplyTo":"1d11d5da7ebc3a6e0d4ab64802b601526c19113c.1401482787.git.jmmahler@gmail.com","subject":"Re: [PATCH v2 2/2] connect.c: replace signal() with sigaction()","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2014-05-31T10:39:39Z","receivedAt":"2014-05-31T10:39:39Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 31/05/14 08:58, Jeremiah Mahler wrote:\n> From signal(2) man page:\n> \n>   The behavior of signal() varies across UNIX versions, and has also var‐\n>   ied historically across different versions of Linux.   Avoid  its  use:\n>   use sigaction(2) instead.\n> \n> Replaced signal() with sigaction() in connect.c\n> \n> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n> ---\n>  connect.c | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/connect.c b/connect.c\n> index a983d06..b2a33c9 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -665,11 +665,14 @@ struct child_process *git_connect(int fd[2], const char *url,\n>  \tenum protocol protocol;\n>  \tconst char **arg;\n>  \tstruct strbuf cmd = STRBUF_INIT;\n> +\tstruct sigaction sa;\n>  \n>  \t/* Without this we cannot rely on waitpid() to tell\n>  \t * what happened to our children.\n>  \t */\n> -\tsignal(SIGCHLD, SIG_DFL);\n> +\tmemset(&sa, 0, sizeof(sa));\n> +\tsa.sa_handler = SIG_DFL;\n> +\tsigaction(SIGCHLD, &sa, 0);\n\nI think this got lost in the wash with v1 but\nDocumentation/CodingGuidelines says to use NULL here instead of 0.\n\n>  \n>  \tprotocol = parse_connect_url(url, &hostandport, &path);\n>  \tif (flags & CONNECT_DIAG_URL) {\n> \n"},{"id":"243077","messageId":"20140531144516.GA22508@hudson.localdomain","threadId":"36798","inReplyTo":"5389B16B.6070004@gmail.com","subject":"Re: [PATCH v2 2/2] connect.c: replace signal() with sigaction()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-31T14:45:16Z","receivedAt":"2014-05-31T14:45:16Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Chris,\n\nOn Sat, May 31, 2014 at 10:39:39PM +1200, Chris Packham wrote:\n> On 31/05/14 08:58, Jeremiah Mahler wrote:\n> > From signal(2) man page:\n> > \n...\n> > -\tsignal(SIGCHLD, SIG_DFL);\n> > +\tmemset(&sa, 0, sizeof(sa));\n> > +\tsa.sa_handler = SIG_DFL;\n> > +\tsigaction(SIGCHLD, &sa, 0);\n> \n> I think this got lost in the wash with v1 but\n> Documentation/CodingGuidelines says to use NULL here instead of 0.\n> \nGot it.  Will be in to the next revision.\n\n  sigaction(SIGCHLD, &sa, NULL);\n\nThanks,\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"}]}