ping: fix write-after-allocated-mem bug

ping: use monotonic_us instead of gettimeofday: smaller code and
      needs only 4 bytes in the packet
ping: display roundtrip times with 1/1000th of ms, not 1/10 ms precision.
wget: small optimization

function                                             old     new   delta
pingstats                                            243     259     +16
sendping6                                             98      93      -5
sendping4                                            183     178      -5
.rodata                                           129715  129707      -8
progressmeter                                        867     855     -12
unpack_tail                                          320     272     -48
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/5 up/down: 16/-78)            Total: -62 bytes
This commit is contained in:
Denis Vlasenko 2007-06-18 08:55:57 +00:00
parent ebd27aabaa
commit 7679145cfa
2 changed files with 32 additions and 37 deletions

View File

@ -242,8 +242,8 @@ struct globals {
int if_index; int if_index;
unsigned long ntransmitted, nreceived, nrepeats, pingcount; unsigned long ntransmitted, nreceived, nrepeats, pingcount;
uint16_t myid; uint16_t myid;
unsigned tmin, tmax; unsigned tmin, tmax; /* in us */
unsigned long tsum; unsigned long long tsum; /* in us, sum of all times */
const char *hostname; const char *hostname;
const char *dotted; const char *dotted;
union { union {
@ -301,11 +301,13 @@ static void pingstats(int junk ATTRIBUTE_UNUSED)
if (ntransmitted) if (ntransmitted)
ntransmitted = (ntransmitted - nreceived) * 100 / ntransmitted; ntransmitted = (ntransmitted - nreceived) * 100 / ntransmitted;
printf("%lu%% packet loss\n", ntransmitted); printf("%lu%% packet loss\n", ntransmitted);
if (tmin != UINT_MAX) if (tmin != UINT_MAX) {
printf("round-trip min/avg/max = %u.%u/%lu.%lu/%u.%u ms\n", unsigned tavg = tsum / (nreceived + nrepeats);
tmin / 10, tmin % 10, printf("round-trip min/avg/max = %u.%03u/%u.%03u/%u.%03u ms\n",
(tsum / (nreceived + nrepeats)) / 10, tmin / 1000, tmin % 1000,
(tsum / (nreceived + nrepeats)) % 10, tmax / 10, tmax % 10); tavg / 1000, tavg % 1000,
tmax / 1000, tmax % 1000);
}
exit(nreceived == 0); /* (nreceived == 0) is true (1) -- 'failure' */ exit(nreceived == 0); /* (nreceived == 0) is true (1) -- 'failure' */
} }
@ -334,7 +336,9 @@ static void sendping_tail(void (*sp)(int), const void *pkt, int size_pkt)
static void sendping4(int junk ATTRIBUTE_UNUSED) static void sendping4(int junk ATTRIBUTE_UNUSED)
{ {
struct icmp *pkt = alloca(datalen + ICMP_MINLEN); /* +4 reserves a place for timestamp, which may end up sitting
* *after* packet. Saves one if() */
struct icmp *pkt = alloca(datalen + ICMP_MINLEN + 4);
pkt->icmp_type = ICMP_ECHO; pkt->icmp_type = ICMP_ECHO;
pkt->icmp_code = 0; pkt->icmp_code = 0;
@ -342,10 +346,9 @@ static void sendping4(int junk ATTRIBUTE_UNUSED)
pkt->icmp_seq = htons(ntransmitted); /* don't ++ here, it can be a macro */ pkt->icmp_seq = htons(ntransmitted); /* don't ++ here, it can be a macro */
pkt->icmp_id = myid; pkt->icmp_id = myid;
// I can't fucking believe someone thought it's okay to do it like this... /* We don't do hton, because we will read it back on the same machine */
// where's hton? Where is a provision for different word size, structure padding, etc?? /*if (datalen >= 4)*/
// FIXME! *(uint32_t*)&pkt->icmp_dun = monotonic_us();
gettimeofday((struct timeval *) &pkt->icmp_dun, NULL);
pkt->icmp_cksum = in_cksum((unsigned short *) pkt, datalen + ICMP_MINLEN); pkt->icmp_cksum = in_cksum((unsigned short *) pkt, datalen + ICMP_MINLEN);
@ -354,7 +357,7 @@ static void sendping4(int junk ATTRIBUTE_UNUSED)
#if ENABLE_PING6 #if ENABLE_PING6
static void sendping6(int junk ATTRIBUTE_UNUSED) static void sendping6(int junk ATTRIBUTE_UNUSED)
{ {
struct icmp6_hdr *pkt = alloca(datalen + sizeof(struct icmp6_hdr)); struct icmp6_hdr *pkt = alloca(datalen + sizeof(struct icmp6_hdr) + 4);
pkt->icmp6_type = ICMP6_ECHO_REQUEST; pkt->icmp6_type = ICMP6_ECHO_REQUEST;
pkt->icmp6_code = 0; pkt->icmp6_code = 0;
@ -362,8 +365,8 @@ static void sendping6(int junk ATTRIBUTE_UNUSED)
pkt->icmp6_seq = htons(ntransmitted); /* don't ++ here, it can be a macro */ pkt->icmp6_seq = htons(ntransmitted); /* don't ++ here, it can be a macro */
pkt->icmp6_id = myid; pkt->icmp6_id = myid;
// FIXME! /*if (datalen >= 4)*/
gettimeofday((struct timeval *) &pkt->icmp6_data8[4], NULL); *(uint32_t*)(&pkt->icmp6_data8[4]) = monotonic_us();
sendping_tail(sendping6, pkt, datalen + sizeof(struct icmp6_hdr)); sendping_tail(sendping6, pkt, datalen + sizeof(struct icmp6_hdr));
} }
@ -417,7 +420,7 @@ static const char *icmp6_type_name(int id)
} }
#endif #endif
static void unpack_tail(int sz, struct timeval *tp, static void unpack_tail(int sz, uint32_t *tp,
const char *from_str, const char *from_str,
uint16_t recv_seq, int ttl) uint16_t recv_seq, int ttl)
{ {
@ -427,17 +430,9 @@ static void unpack_tail(int sz, struct timeval *tp,
++nreceived; ++nreceived;
if (tp) { if (tp) {
struct timeval tv; /* (int32_t) cast is for hypothetical 64-bit unsigned */
/* (doesn't hurt 32-bit real-world anyway) */
gettimeofday(&tv, NULL); triptime = (int32_t) ((uint32_t)monotonic_us() - *tp);
tv.tv_usec -= tp->tv_usec;
if (tv.tv_usec < 0) {
--tv.tv_sec;
tv.tv_usec += 1000000;
}
tv.tv_sec -= tp->tv_sec;
triptime = tv.tv_sec * 10000 + (tv.tv_usec / 100);
tsum += triptime; tsum += triptime;
if (triptime < tmin) if (triptime < tmin)
tmin = triptime; tmin = triptime;
@ -459,7 +454,7 @@ static void unpack_tail(int sz, struct timeval *tp,
printf("%d bytes from %s: seq=%u ttl=%d", sz, printf("%d bytes from %s: seq=%u ttl=%d", sz,
from_str, recv_seq, ttl); from_str, recv_seq, ttl);
if (tp) if (tp)
printf(" time=%u.%u ms", triptime / 10, triptime % 10); printf(" time=%u.%03u ms", triptime / 1000, triptime % 1000);
puts(dupmsg); puts(dupmsg);
fflush(stdout); fflush(stdout);
} }
@ -483,10 +478,10 @@ static void unpack4(char *buf, int sz, struct sockaddr_in *from)
if (icmppkt->icmp_type == ICMP_ECHOREPLY) { if (icmppkt->icmp_type == ICMP_ECHOREPLY) {
uint16_t recv_seq = ntohs(icmppkt->icmp_seq); uint16_t recv_seq = ntohs(icmppkt->icmp_seq);
struct timeval *tp = NULL; uint32_t *tp = NULL;
if (sz >= ICMP_MINLEN + sizeof(struct timeval)) if (sz >= ICMP_MINLEN + sizeof(uint32_t))
tp = (struct timeval *) icmppkt->icmp_data; tp = (uint32_t *) icmppkt->icmp_data;
unpack_tail(sz, tp, unpack_tail(sz, tp,
inet_ntoa(*(struct in_addr *) &from->sin_addr.s_addr), inet_ntoa(*(struct in_addr *) &from->sin_addr.s_addr),
recv_seq, iphdr->ttl); recv_seq, iphdr->ttl);
@ -512,10 +507,10 @@ static void unpack6(char *packet, int sz, struct sockaddr_in6 *from, int hoplimi
if (icmppkt->icmp6_type == ICMP6_ECHO_REPLY) { if (icmppkt->icmp6_type == ICMP6_ECHO_REPLY) {
uint16_t recv_seq = ntohs(icmppkt->icmp6_seq); uint16_t recv_seq = ntohs(icmppkt->icmp6_seq);
struct timeval *tp = NULL; uint32_t *tp = NULL;
if (sz >= sizeof(struct icmp6_hdr) + sizeof(struct timeval)) if (sz >= sizeof(struct icmp6_hdr) + sizeof(uint32_t))
tp = (struct timeval *) &icmppkt->icmp6_data8[4]; tp = (uint32_t *) &icmppkt->icmp6_data8[4];
unpack_tail(sz, tp, unpack_tail(sz, tp,
inet_ntop(AF_INET6, &pingaddr.sin6.sin6_addr, inet_ntop(AF_INET6, &pingaddr.sin6.sin6_addr,
buf, sizeof(buf)), buf, sizeof(buf)),

View File

@ -8,7 +8,7 @@
/* We want libc to give us xxx64 functions also */ /* We want libc to give us xxx64 functions also */
/* http://www.unix.org/version2/whatsnew/lfs20mar.html */ /* http://www.unix.org/version2/whatsnew/lfs20mar.html */
#define _LARGEFILE64_SOURCE 1 //#define _LARGEFILE64_SOURCE 1
#include <getopt.h> /* for struct option */ #include <getopt.h> /* for struct option */
#include "libbb.h" #include "libbb.h"
@ -710,7 +710,7 @@ progressmeter(int flag)
fprintf(stderr, "\r%-20.20s%4d%% ", curfile, ratio); fprintf(stderr, "\r%-20.20s%4d%% ", curfile, ratio);
barlength = getttywidth() - 51; barlength = getttywidth() - 49;
if (barlength > 0) { if (barlength > 0) {
/* god bless gcc for variable arrays :) */ /* god bless gcc for variable arrays :) */
i = barlength * ratio / 100; i = barlength * ratio / 100;
@ -728,7 +728,7 @@ progressmeter(int flag)
abbrevsize >>= 10; abbrevsize >>= 10;
} }
/* see http://en.wikipedia.org/wiki/Tera */ /* see http://en.wikipedia.org/wiki/Tera */
fprintf(stderr, "%6d %c%c ", (int)abbrevsize, " KMGTPEZY"[i], i?'B':' '); fprintf(stderr, "%6d%c ", (int)abbrevsize, " kMGTPEZY"[i]);
// Nuts! Ain't it easier to update progress meter ONLY when we transferred++? // Nuts! Ain't it easier to update progress meter ONLY when we transferred++?
// FIXME: get rid of alarmtimer + updateprogressmeter mess // FIXME: get rid of alarmtimer + updateprogressmeter mess