Thanks Florian & team.
Please review the following diff.
Index: ping.c
===================================================================
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.100
diff -u -p -u -r1.100 ping.c
--- ping.c 24 Mar 2014 11:11:49 -0000 1.100
+++ ping.c 22 Apr 2014 19:00:28 -0000
@@ -72,6 +72,7 @@
#include <errno.h>
#include <string.h>
#include <stdlib.h>
+#include <poll.h>
struct tv32 {
u_int tv32_sec;
@@ -175,7 +176,7 @@ void usage(void);
int
main(int argc, char *argv[])
{
- struct timeval timeout;
+ int timeout;
struct hostent *hp;
struct sockaddr_in *to;
struct in_addr saddr;
@@ -187,8 +188,7 @@ main(int argc, char *argv[])
#endif
socklen_t maxsizelen;
const char *errstr;
- fd_set *fdmaskp;
- size_t fdmasks;
+ struct pollfd fdmaskp[1];
uid_t uid;
u_int rtableid;
@@ -507,10 +507,6 @@ main(int argc, char *argv[])
if ((options & F_FLOOD) == 0)
catcher(0); /* start things going */
- fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
- if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
- err(1, "malloc");
-
for (;;) {
struct sockaddr_in from;
sigset_t omask, nmask;
@@ -519,13 +515,19 @@ main(int argc, char *argv[])
if (options & F_FLOOD) {
pinger();
- timeout.tv_sec = 0;
- timeout.tv_usec = 10000;
- memset(fdmaskp, 0, fdmasks);
- FD_SET(s, fdmaskp);
- if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
- (fd_set *)NULL, &timeout) < 1)
- continue;
+ timeout = 10;
+ fdmaskp[0].fd = s;
+ fdmaskp[0].events = POLLIN;
+ cc = poll(fdmaskp, 1, timeout);
+
+ if (cc < 0) {
+ if (errno != EINTR) {
+ warn("poll");
+ sleep(1);
+ }
+ continue;
+ } else if (cc == 0)
+ continue;
}
fromlen = sizeof(from);
if ((cc = recvfrom(s, packet, packlen, 0,
@@ -543,7 +545,6 @@ main(int argc, char *argv[])
if (npackets && nreceived >= npackets)
break;
}
- free(fdmaskp);
finish(0);
/* NOTREACHED */
exit(0); /* Make the compiler happy */
Quoting Florian Obser <flor...@openbsd.org>:
Please switch it to poll(2) like ping6(8) is doing, there by side
stepping the whole issue.
On Tue, Apr 22, 2014 at 09:33:50AM +0200, Otto Moerbeek wrote:
On Tue, Apr 22, 2014 at 02:57:54AM -0400, pe...@petermalone.org wrote:
> Sure - I should have spotted that.
Still not there. Please use the fact that calloc can multiply, you get
an overflow check for free.
-Otto
>
> Index: ping.c
> ===================================================================
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.100
> diff -u -r1.100 ping.c
> --- ping.c 24 Mar 2014 11:11:49 -0000 1.100
> +++ ping.c 22 Apr 2014 06:55:03 -0000
> @@ -188,7 +188,6 @@
> socklen_t maxsizelen;
> const char *errstr;
> fd_set *fdmaskp;
> - size_t fdmasks;
> uid_t uid;
> u_int rtableid;
>
> @@ -507,9 +506,8 @@
> if ((options & F_FLOOD) == 0)
> catcher(0); /* start things going */
>
> - fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> - if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> - err(1, "malloc");
> + if ((fdmaskp = calloc(1, howmany(s+1, NFDBITS) * sizeof(fd_mask)))
> == NULL)
> + err(1, "calloc");
>
> for (;;) {
> struct sockaddr_in from;
> @@ -521,7 +519,6 @@
> pinger();
> timeout.tv_sec = 0;
> timeout.tv_usec = 10000;
> - memset(fdmaskp, 0, fdmasks);
> FD_SET(s, fdmaskp);
> if (select(s + 1, (fd_set *)fdmaskp,
(fd_set *)NULL,
> (fd_set *)NULL, &timeout) < 1)
>
>
> Quoting Otto Moerbeek <o...@drijf.net>:
>
> >On Tue, Apr 22, 2014 at 12:45:25AM -0400, Peter Malone wrote:
> >
> >>Hi,
> >>
> >>malloc & memset can be replaced with calloc in ping.c. Please
see below for
> >>patch details:
> >
> >Better rework this to get rid of fdmasks.
> >
> > -Otto
> >
> >>
> >>Index: ping.c
> >>===================================================================
> >>RCS file: /cvs/src/sbin/ping/ping.c,v
> >>retrieving revision 1.100
> >>diff -u -p -u -r1.100 ping.c
> >>--- ping.c 24 Mar 2014 11:11:49 -0000 1.100
> >>+++ ping.c 22 Apr 2014 04:41:56 -0000
> >>@@ -508,8 +508,8 @@ main(int argc, char *argv[])
> >> catcher(0); /* start things going */
> >>
> >> fdmasks = howmany(s+1, NFDBITS) * sizeof(fd_mask);
> >>- if ((fdmaskp = (fd_set *)malloc(fdmasks)) == NULL)
> >>- err(1, "malloc");
> >>+ if ((fdmaskp = calloc(1, fdmasks)) == NULL)
> >>+ err(1, "calloc");
> >>
> >> for (;;) {
> >> struct sockaddr_in from;
> >>@@ -521,7 +521,6 @@ main(int argc, char *argv[])
> >> pinger();
> >> timeout.tv_sec = 0;
> >> timeout.tv_usec = 10000;
> >>- memset(fdmaskp, 0, fdmasks);
> >> FD_SET(s, fdmaskp);
> >> if (select(s + 1, (fd_set *)fdmaskp, (fd_set *)NULL,
> >> (fd_set *)NULL, &timeout) < 1)
> >>
> >
> >
>
--
I'm not entirely sure you are real.
