![[tokkee]](http://tokkee.org/images/avatar.png){kind=avatar}
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 4ef7c73)
raw | patch | inline | side by side (parent: 4ef7c73)
| author | Florian Forster <ff@octo.it> | |
| Fri, 5 May 2017 08:42:35 +0000 (10:42 +0200) | ||
| committer | Florian Forster <ff@octo.it> | |
| Fri, 5 May 2017 08:47:32 +0000 (10:47 +0200) |
* Give more meaningful names to the central "ptr" and "pings" variables
(now "host_to_ping" and "pings_in_flight").
* Remove special case for (errno == EINTR); effectively we only printed a
different debugging message in that case.
* Remove err_fds. We were not checking it at all.
* Simplify the logic so only one receive or send operation happens per
loop iteration. Previously, one IPv4 and one IPv6 operation might happen
in the same loop iteration. The new logic always receives all replies
before starting to send out more requests.
* Assign the write file descriptor to its own variable to make clear that
only file descriptor is set in the write_fds bitmask.
(now "host_to_ping" and "pings_in_flight").
* Remove special case for (errno == EINTR); effectively we only printed a
different debugging message in that case.
* Remove err_fds. We were not checking it at all.
* Simplify the logic so only one receive or send operation happens per
loop iteration. Previously, one IPv4 and one IPv6 operation might happen
in the same loop iteration. The new logic always receives all replies
before starting to send out more requests.
* Assign the write file descriptor to its own variable to make clear that
only file descriptor is set in the write_fds bitmask.
| src/liboping.c | patch | blob | history |
diff --git a/src/liboping.c b/src/liboping.c
index ad788ed144328b5bda859315f14dace34cde277e..fd6e94227e4f7ff68d50c8eff308ea9d35a7afd4 100644 (file)
--- a/src/liboping.c
+++ b/src/liboping.c
struct timeval nowtime;
struct timeval timeout;
- /* pings is the number of in-flight pings, i.e. hosts we sent a "ping"
- * to but didn't receive a "pong" yet. */
- int pings = 0;
int ret = 0;
_Bool need_ipv4_socket = 0;
ping_timeval_add (&nowtime, &timeout, &endtime);
- ptr = obj->head;
- while (pings > 0 || ptr != NULL)
+ /* host_to_ping points to the host to which to send the next ping. The
+ * pointer is advanced to the next host in the linked list after the
+ * ping has been sent. If host_to_ping is NULL, no more pings need to be
+ * send out. */
+ pinghost_t *host_to_ping = obj->head;
+
+ /* pings_in_flight is the number of hosts we sent a "ping" to but didn't
+ * receive a "pong" yet. */
+ int pings_in_flight = 0;
+
+ while (pings_in_flight > 0 || host_to_ping != NULL)
{
fd_set read_fds;
fd_set write_fds;
- fd_set err_fds;
+ int write_fd = -1;
int max_fd = -1;
FD_ZERO (&read_fds);
FD_ZERO (&write_fds);
- FD_ZERO (&err_fds);
if (obj->fd4 != -1)
{
FD_SET(obj->fd4, &read_fds);
- if (ptr != NULL && ptr->addrfamily == AF_INET)
- FD_SET(obj->fd4, &write_fds);
- FD_SET(obj->fd4, &err_fds);
+ if (host_to_ping != NULL && host_to_ping->addrfamily == AF_INET)
+ write_fd = obj->fd4;
if (max_fd < obj->fd4)
max_fd = obj->fd4;
if (obj->fd6 != -1)
{
FD_SET(obj->fd6, &read_fds);
- if (ptr != NULL && ptr->addrfamily == AF_INET6)
- FD_SET(obj->fd6, &write_fds);
- FD_SET(obj->fd6, &err_fds);
+ if (host_to_ping != NULL && host_to_ping->addrfamily == AF_INET6)
+ write_fd = obj->fd6;
if (max_fd < obj->fd6)
max_fd = obj->fd6;
}
+ if (write_fd != -1)
+ FD_SET(write_fd, &write_fds);
+
assert (max_fd != -1);
assert (max_fd < FD_SETSIZE);
(int) timeout.tv_sec,
(int) timeout.tv_usec);
- int status = select (max_fd + 1, &read_fds, &write_fds, &err_fds, &timeout);
+ int status = select (max_fd + 1, &read_fds, &write_fds, NULL, &timeout);
if (gettimeofday (&nowtime, NULL) == -1)
{
return (-1);
}
- if ((status == -1) && (errno == EINTR))
+ if (status == -1)
{
- dprintf ("select was interrupted by signal..\n");
- ping_set_errno (obj, EINTR);
+ ping_set_errno (obj, errno);
+ dprintf ("select: %s\n", obj->errmsg);
return (-1);
}
- else if (status < 0)
- {
-#if WITH_DEBUG
- char errbuf[PING_ERRMSG_LEN];
- dprintf ("select: %s\n",
- sstrerror (errno, errbuf, sizeof (errbuf)));
-#endif
- break;
- }
else if (status == 0)
{
dprintf ("select timed out\n");
- for (ptr = obj->head; ptr != NULL; ptr = ptr->next)
- if (ptr->latency < 0.0)
- ptr->dropped++;
+ pinghost_t *ph;
+ for (ph = obj->head; ph != NULL; ph = ph->next)
+ if (ph->latency < 0.0)
+ ph->dropped++;
break;
}
- if (obj->fd4 != -1)
+ /* first, check if we can receive a reply ... */
+ if (obj->fd6 != -1 && FD_ISSET (obj->fd6, &read_fds))
{
- if (FD_ISSET (obj->fd4, &read_fds))
- {
- if (!ping_receive_one(obj, &nowtime, AF_INET))
- --pings;
- }
- else if (ptr != NULL && ptr->addrfamily == AF_INET &&
- FD_ISSET (obj->fd4, &write_fds))
- {
- if (!ping_send_one(obj, ptr, obj->fd4))
- {
- ptr = ptr->next;
- ++pings;
- }
- else
- {
- --ret;
- }
- }
-
+ if (ping_receive_one (obj, &nowtime, AF_INET6) == 0)
+ pings_in_flight--;
+ continue;
+ }
+ if (obj->fd4 != -1 && FD_ISSET (obj->fd4, &read_fds))
+ {
+ if (ping_receive_one (obj, &nowtime, AF_INET) == 0)
+ pings_in_flight--;
+ continue;
}
- if (obj->fd6 != -1)
+ /* ... and if no reply is available to read, continue sending
+ * out pings. */
+
+ /* this condition should always be true. We keep it for
+ * consistency with the read blocks above and just to be on the
+ * safe side. */
+ if (write_fd != -1 && FD_ISSET (write_fd, &write_fds))
{
- if (FD_ISSET (obj->fd6, &read_fds))
- {
- if (!ping_receive_one(obj, &nowtime, AF_INET6))
- --pings;
- }
- else if (ptr != NULL && ptr->addrfamily == AF_INET6 &&
- FD_ISSET (obj->fd6, &write_fds))
- {
- if (!ping_send_one(obj, ptr, obj->fd6))
- {
- ++pings;
- ptr = ptr->next;
- }
- else
- {
- --ret;
- }
- }
+ if (ping_send_one (obj, host_to_ping, write_fd) == 0)
+ pings_in_flight++;
+ else
+ ret--;
+ host_to_ping = host_to_ping->next;
+ continue;
}
} /* while (1) */