Re: [PATCH v3 0/9] replace signal() with sigaction()
On Mon, Jun 02, 2014 at 12:05:29PM -0700, Junio C Hamano wrote: > Johannes Sixt writes: > > >> Jeremiah Mahler (9): > >> compat/mingw.c: expand MinGW support for sigaction > >> connect.c: replace signal() with sigaction() > >> progress.c: replace signal() with sigaction() > >> write_or_die.c: replace signal() with sigaction() > >> daemon.c: replace signal() with sigaction() > >> builtin/log.c: replace signal() with sigaction() > >> builtin/merge-index.c: replace signal() with sigaction() > >> builtin/verify-tag.c: replace signal() with sigaction() > >> sigchain.c: replace signal() with sigaction() > > > > The series without patch 9/9 works on Windows so far. > > > > Without patch patch 9/9 and a more complete implementation of sigaction in > > compat/mingw.c the series misses its goal. But even if you complete it, it > > is IMHO only code churn without practical merits. > > Hmm, you sound a bit harsher than you usually do---although I > sort of share with you the doubt on the practical merits. > Alright, I'm dropping it. Too much work for no real gain other than some piece of mind. Thanks Johannes and Junio for your feedback. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] replace signal() with sigaction()
Johannes Sixt writes: >> Jeremiah Mahler (9): >> compat/mingw.c: expand MinGW support for sigaction >> connect.c: replace signal() with sigaction() >> progress.c: replace signal() with sigaction() >> write_or_die.c: replace signal() with sigaction() >> daemon.c: replace signal() with sigaction() >> builtin/log.c: replace signal() with sigaction() >> builtin/merge-index.c: replace signal() with sigaction() >> builtin/verify-tag.c: replace signal() with sigaction() >> sigchain.c: replace signal() with sigaction() > > The series without patch 9/9 works on Windows so far. > > Without patch patch 9/9 and a more complete implementation of sigaction in > compat/mingw.c the series misses its goal. But even if you complete it, it > is IMHO only code churn without practical merits. Hmm, you sound a bit harsher than you usually do---although I sort of share with you the doubt on the practical merits. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] replace signal() with sigaction()
Hannes, On Mon, Jun 02, 2014 at 01:28:25PM +0200, Johannes Sixt wrote: > Am 6/1/2014 20:10, schrieb Jeremiah Mahler: > > This is version 3 of the patch set to convert signal(2) to sigaction(2) > > (previous discussion [1]). > > ... > > sigchain.c: replace signal() with sigaction() > > The series without patch 9/9 works on Windows so far. > > Without patch patch 9/9 and a more complete implementation of sigaction in > compat/mingw.c the series misses its goal. But even if you complete it, it > is IMHO only code churn without practical merits. > > -- Hannes > You are right, I missed the case where the old signal was used, as is done in sigchain.c. Sorry about that. Thanks again for looking at my patch. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] replace signal() with sigaction()
Am 6/1/2014 20:10, schrieb Jeremiah Mahler: > This is version 3 of the patch set to convert signal(2) to sigaction(2) > (previous discussion [1]). > > [1]: http://marc.info/?l=git&m=140148352416926&w=2 > > Changes in this revision include: > > - Using NULL pointers instead of 0 as per the > Documentation/CodingGuidlines pointed out by Chris Packham. > > sigaction(SIGCHLD, &sa, NULL); > > - Conversion of all remaining files which used signal(). > > - sigchain.c required the most changes. Both the old signal handler > was used and the return value from signal() was being checked. > signal() would return the previous error handler which would be > SIG_ERR if an error occurred. sigaction() just returns -1 in this > case. > > Jeremiah Mahler (9): > compat/mingw.c: expand MinGW support for sigaction > connect.c: replace signal() with sigaction() > progress.c: replace signal() with sigaction() > write_or_die.c: replace signal() with sigaction() > daemon.c: replace signal() with sigaction() > builtin/log.c: replace signal() with sigaction() > builtin/merge-index.c: replace signal() with sigaction() > builtin/verify-tag.c: replace signal() with sigaction() > sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. -- Hannes > > builtin/log.c | 6 +- > builtin/merge-index.c | 5 - > builtin/verify-tag.c | 5 - > compat/mingw.c| 9 + > connect.c | 5 - > daemon.c | 16 +--- > progress.c| 6 +- > sigchain.c| 14 +++--- > write_or_die.c| 6 +- > 9 files changed, 56 insertions(+), 16 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/9] replace signal() with sigaction()
This is version 3 of the patch set to convert signal(2) to sigaction(2) (previous discussion [1]). [1]: http://marc.info/?l=git&m=140148352416926&w=2 Changes in this revision include: - Using NULL pointers instead of 0 as per the Documentation/CodingGuidlines pointed out by Chris Packham. sigaction(SIGCHLD, &sa, NULL); - Conversion of all remaining files which used signal(). - sigchain.c required the most changes. Both the old signal handler was used and the return value from signal() was being checked. signal() would return the previous error handler which would be SIG_ERR if an error occurred. sigaction() just returns -1 in this case. Jeremiah Mahler (9): compat/mingw.c: expand MinGW support for sigaction connect.c: replace signal() with sigaction() progress.c: replace signal() with sigaction() write_or_die.c: replace signal() with sigaction() daemon.c: replace signal() with sigaction() builtin/log.c: replace signal() with sigaction() builtin/merge-index.c: replace signal() with sigaction() builtin/verify-tag.c: replace signal() with sigaction() sigchain.c: replace signal() with sigaction() builtin/log.c | 6 +- builtin/merge-index.c | 5 - builtin/verify-tag.c | 5 - compat/mingw.c| 9 + connect.c | 5 - daemon.c | 16 +--- progress.c| 6 +- sigchain.c| 14 +++--- write_or_die.c| 6 +- 9 files changed, 56 insertions(+), 16 deletions(-) -- 2.0.0.8.g7bf6e1f.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
