svn commit: r273295 - in head/sbin: ping ping6
Hiroki Sato
hrs at FreeBSD.org
Mon Oct 20 07:33:04 UTC 2014
Hi Bruce,
Thank you for your review. I fixed lines you pointed out and
attached a patch candidate. I will show each change line by line in
this email, too:
Bruce Evans <brde at optusnet.com.au> wrote
in <20141020140848.D935 at besplex.bde.org>:
br> > - if (cc - ICMP_MINLEN - phdr_len >= sizeof(tv1)) {
br> > + if ((size_t)(cc - ICMP_MINLEN - phdr_len) >=
br> > + sizeof(tv1)) {
br> > /* Copy to avoid alignment problems: */
br> > memcpy(&tv32, tp, sizeof(tv32));
br> > tv1.tv_sec = ntohl(tv32.tv32_sec);
br>
br> This breaks the warning, and breaks the code on exotic (unsupported)
br> where it used to work accidentally. The code was already broken on
br> non-exotic arches.
- if ((size_t)(cc - ICMP_MINLEN - phdr_len) >=
- sizeof(tv1)) {
+ if (cc - ICMP_MINLEN - phdr_len >= (int)sizeof(tv1)) {
br> > -struct sockaddr_in6 dst; /* who to ping6 */
br> > -struct sockaddr_in6 src; /* src addr of this packet */
br> > -socklen_t srclen;
br>
br> Old code like ping uses plain int for almost everything except where
br> the broken ABI requires an unsigned type like socklen_t.
-static socklen_t srclen;
-static size_t datalen = DEFDATALEN;
+static int srclen;
+static int datalen = DEFDATALEN;
br> > +static u_int8_t nonce[8]; /* nonce field for node information */
br>
br> This uses the deprecated nonstandard spelling of uint8_t.
s/u_int_{8,16,32}_t/uint_{8,16,32}_t/ in various places.
br> > -void pr_suptypes(struct icmp6_nodeinfo *, size_t);
s/size_t/int/
br> > - sockbufsize = lsockbufsize;
br> > + sockbufsize = (int)lsockbufsize;
br> > if (errno || !*optarg || *e ||
br> > - sockbufsize != lsockbufsize)
br> > + lsockbufsize > INT_MAX)
br> > errx(1, "invalid socket buffer size");
br> I don't like warnings about hackish but correct code like the previous
br> version of the above. Does the compiler actually complain about
br> comparing
br> ints with unsigned longs for equality?
Yes, this part caused the following warning:
/usr/src/head/sbin/ping6/ping6.c:395:20: error: comparison of integers of
different signs: 'int' and 'u_long' (aka 'unsigned long')
[-Werror,-Wsign-compare]
sockbufsize != lsockbufsize)
br> If you are going to change the range check, then rearrange the code.
br> It
br> was arranged to set sockbufsize before the value is known to fit so
br> that
br> non-fitting values can be checked. With range checking on the
br>
br> original
br> value, it is more natural to not do the assignment until after the
br> check has passed. Then the cast should be unnecessary since a smart
br> compiler would see from the range check that the assignment can't
br> overflow.
- lsockbufsize = strtoul(optarg, &e, 10);
- sockbufsize = (int)lsockbufsize;
- if (errno || !*optarg || *e ||
- lsockbufsize > INT_MAX)
+ ultmp = strtoul(optarg, &e, 10);
+ if (errno || !*optarg || *e || ultmp > INT_MAX)
errx(1, "invalid socket buffer size");
+ sockbufsize = ultmp;
br> > - if (l >= sizeof(cresult) || l < 0)
br> > + if ((size_t)l >= sizeof(cresult) || l < 0)
br>
br> The conversion is backwards.
br>
- if ((size_t)l >= sizeof(cresult) || l < 0)
+ if (l >= (int)sizeof(cresult) || l < 0)
br> > - if (end - (u_char *)ip6 < sizeof(*ip6)) {
br> > + if ((size_t)(end - (u_char *)ip6) < sizeof(*ip6)) {
br> > printf("IP6");
br> > goto trunc;
br> > }
br>
br> Backwards. The RHS should be cast to ptrdiff_t. Pointer difference
br> are only required to work up to 64K, but that is enough for the size
br> of small objects.
- if ((size_t)(end - (u_char *)ip6) < sizeof(*ip6)) {
+ if (end - (u_char *)ip6 < (ptrdiff_t)sizeof(*ip6)) {
br> > - kk <= MAXDATALEN - (8 + sizeof(struct tv32) + ii);
br> > + (size_t)kk <= MAXDATALEN - 8 + sizeof(struct tv32) + ii;
- (size_t)kk <= MAXDATALEN - 8 + sizeof(struct tv32) + ii;
+ kk <= MAXDATALEN - 8 + (int)sizeof(struct tv32) + ii;
-- Hiroki
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ping6.c_20141020-1.diff
Type: text/x-patch
Size: 12854 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20141020/e9f91ba3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ping.c_20141020-1.diff
Type: text/x-patch
Size: 460 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20141020/e9f91ba3/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20141020/e9f91ba3/attachment.sig>
More information about the svn-src-all
mailing list