tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Signed vs unsigned comparisons in sys/dev/pcio/if_wm.c



   Date: Sun, 13 Nov 2016 09:33:53 +0800 (PHT)
   From: Paul Goyette <paul%whooppee.com@localhost>

   There are three instances within if_wm.c where this occurs, at source 
   lines 6164, 6688, and 9204 (based on revision 1.443).  The attached 
   diffs make the compiler happy in both cases (kernel and module).

   @@ -438,7 +438,7 @@ struct wm_softc {
           int sc_funcid;                  /* unit number of the chip (0 to 3) */
           int sc_flags;                   /* flags; see below */
           int sc_if_flags;                /* last if_flags */
   -       int sc_flowflags;               /* 802.3x flow control flags */
   +       unsigned int sc_flowflags;      /* 802.3x flow control flags */

Can't say without examining every use of sc_flowflags.  In this case,
it looks like there are other (soft) constraints -- e.g., a cursory
glance suggests it ought to have the same type as struct mii_data::
mii_media_active and struct ifreq::ifr_media.

   @@ -6162,7 +6162,7 @@ wm_tx_offload(struct wm_softc *sc, struc
                   bool v4 = (m0->m_pkthdr.csum_flags & M_CSUM_TSOv4) != 0;

                   if (__predict_false(m0->m_len <
   -                                   (hlen + sizeof(struct tcphdr)))) {
   +                                   (hlen + (int)sizeof(struct tcphdr)))) {

Any (semi-)mechanical translation should include an assertion, or an
argument that such an assertion already exists or is trivially
needless, that the conversion is lossless.

In this case, I would prefer see an assertion that the m_len is
nonnegative and conversion to size_t, rather than an assertion that
the size fits in an int and conversion to int.  Conversion of size_t
to int is a much bigger change on some platforms (loss of 32 bits of
range) than conversion of int to size_t (reinterpretation of one bit
of range), though the difference here doesn't matter much.


Home | Main Index | Thread Index | Old Index