Re: LP64: (int)signal()
On Mon, 1 Jul 2002, Christian Weisgerber wrote: I would like to clean up the last instances of (int)signal(...) in the tree. Any objection to the changes below? No, but ... Other occurrences not worth touching: - contrib/opie/opieftpd.c: contrib, not used - libexec/bootpd/bootpd.c: #ifdef'ed out in favor of sigaction(). ... either the sigaction() or the signal() in bootpd.c is probably wrong, since the sigaction() is missing SA_RESTART. The signal() case has been ifdefed out for a long time, so this apparently doesn't matter much. This has very little to do with LP64 ... Mike Barcroft [EMAIL PROTECTED] wrote: You might want to get rid of the other misuse of `rc' above this and just remove the variable. The use of an gratuitous int variable rc to capture return values is rampant throughout this code. In fact, not using it is something of a violation of the local style, but in the case of signal() I think it's justifiable because SIG_ERR is so much neater than changing rc to long and messing around with explicit casts. Casting signal() to long just gives undefined behaviour on different machines. It would be less incorrect to cast a magic integer to a function pointer like the implementation must do to create SIG_ERR (at least if the implementation starts with a magic integer), but it is so much neater to just use SIG_ERR. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
LP64: (int)signal()
I would like to clean up the last instances of (int)signal(...) in
the tree. Any objection to the changes below?
Other occurrences not worth touching:
- contrib/opie/opieftpd.c: contrib, not used
- libexec/bootpd/bootpd.c: #ifdef'ed out in favor of sigaction().
Index: atmarpd/atmarpd.c
===
RCS file: /home/ncvs/src/usr.sbin/atm/atmarpd/atmarpd.c,v
retrieving revision 1.4
diff -u -r1.4 atmarpd.c
--- atmarpd/atmarpd.c 9 Dec 2000 09:35:42 - 1.4
+++ atmarpd/atmarpd.c 1 Jul 2002 11:38:07 -
@@ -294,8 +294,7 @@
/*
* Set up signal handlers
*/
- rc = (int)signal(SIGINT, atmarp_sigint);
- if (rc == -1) {
+ if (signal(SIGINT, atmarp_sigint) == SIG_ERR) {
atmarp_log(LOG_ERR, SIGINT signal setup failed);
exit(1);
}
Index: scspd/scspd.c
===
RCS file: /home/ncvs/src/usr.sbin/atm/scspd/scspd.c,v
retrieving revision 1.4
diff -u -r1.4 scspd.c
--- scspd/scspd.c 9 Dec 2000 09:35:42 - 1.4
+++ scspd/scspd.c 1 Jul 2002 11:38:08 -
@@ -319,14 +319,12 @@
/*
* Set up signal handlers
*/
- rc = (int)signal(SIGHUP, scsp_sighup);
- if (rc == -1) {
+ if (signal(SIGHUP, scsp_sighup) == SIG_ERR) {
scsp_log(LOG_ERR, SIGHUP signal setup failed);
exit(1);
}
- rc = (int)signal(SIGINT, scsp_sigint);
- if (rc == -1) {
+ if (signal(SIGINT, scsp_sigint) == SIG_ERR) {
scsp_log(LOG_ERR, SIGINT signal setup failed);
exit(1);
}
--
Christian naddy Weisgerber [EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: LP64: (int)signal()
Christian Weisgerber [EMAIL PROTECTED] writes:
I would like to clean up the last instances of (int)signal(...) in
the tree. Any objection to the changes below?
Other occurrences not worth touching:
- contrib/opie/opieftpd.c: contrib, not used
- libexec/bootpd/bootpd.c: #ifdef'ed out in favor of sigaction().
Index: atmarpd/atmarpd.c
===
RCS file: /home/ncvs/src/usr.sbin/atm/atmarpd/atmarpd.c,v
retrieving revision 1.4
diff -u -r1.4 atmarpd.c
--- atmarpd/atmarpd.c 9 Dec 2000 09:35:42 - 1.4
+++ atmarpd/atmarpd.c 1 Jul 2002 11:38:07 -
@@ -294,8 +294,7 @@
/*
* Set up signal handlers
*/
- rc = (int)signal(SIGINT, atmarp_sigint);
- if (rc == -1) {
+ if (signal(SIGINT, atmarp_sigint) == SIG_ERR) {
atmarp_log(LOG_ERR, SIGINT signal setup failed);
exit(1);
}
You might want to get rid of the other misuse of `rc' above this and
just remove the variable.
Index: scspd/scspd.c
===
RCS file: /home/ncvs/src/usr.sbin/atm/scspd/scspd.c,v
retrieving revision 1.4
diff -u -r1.4 scspd.c
--- scspd/scspd.c 9 Dec 2000 09:35:42 - 1.4
+++ scspd/scspd.c 1 Jul 2002 11:38:08 -
@@ -319,14 +319,12 @@
/*
* Set up signal handlers
*/
- rc = (int)signal(SIGHUP, scsp_sighup);
- if (rc == -1) {
+ if (signal(SIGHUP, scsp_sighup) == SIG_ERR) {
scsp_log(LOG_ERR, SIGHUP signal setup failed);
exit(1);
}
- rc = (int)signal(SIGINT, scsp_sigint);
- if (rc == -1) {
+ if (signal(SIGINT, scsp_sigint) == SIG_ERR) {
scsp_log(LOG_ERR, SIGINT signal setup failed);
exit(1);
}
[Repeat above sentence.] :)
Otherwise it looks good.
Best regards,
Mike Barcroft
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: LP64: (int)signal()
Mike Barcroft [EMAIL PROTECTED] wrote: You might want to get rid of the other misuse of `rc' above this and just remove the variable. The use of an gratuitous int variable rc to capture return values is rampant throughout this code. In fact, not using it is something of a violation of the local style, but in the case of signal() I think it's justifiable because SIG_ERR is so much neater than changing rc to long and messing around with explicit casts. -- Christian naddy Weisgerber [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: LP64: (int)signal()
On Mon, 1 Jul 2002, Christian Weisgerber wrote: Mike Barcroft [EMAIL PROTECTED] wrote: You might want to get rid of the other misuse of `rc' above this and just remove the variable. The use of an gratuitous int variable rc to capture return values is rampant throughout this code. In fact, not using it is something of a violation of the local style, but in the case of signal() I think it's justifiable because SIG_ERR is so much neater than changing rc to long and messing around with explicit casts. How about eliminating signal() all together and using sigaction()? -- Dan Eischen To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
