On Tue, Jan 07, 2014 at 06:40:47PM +0000, Williams, Mitch A wrote: > > > > -----Original Message----- > > From: Dan Carpenter [mailto:[email protected]] > > Sent: Monday, January 06, 2014 10:12 PM > > To: Rose, Gregory V > > Cc: [email protected]; Williams, Mitch A > > Subject: re: i40evf: main driver core > > > > Hello Greg Rose, > > > > The patch 5eae00c57f5e: "i40evf: main driver core" from Dec 21, 2013, > > leads to the following static checker warning: > > "drivers/net/ethernet/intel/i40evf/i40evf_main.c:587 i40evf_configure_rx() > > warn: was expecting a 64 bit value instead of '(1 << 2)'" > > > > drivers/net/ethernet/intel/i40evf/i40evf_main.c > > 586 > > 587 adapter->flags &= ~I40EVF_FLAG_RX_PS_CAPABLE; > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > flags is unsigned long but the bitwise negate is on 32 bit so it clears > > the upper 32 bits in a potentially uninintended way. > > > > 588 adapter->flags |= I40EVF_FLAG_RX_1BUF_CAPABLE; > > 589 > > > > drivers/net/ethernet/intel/i40evf/i40evf.h > > 228 volatile unsigned long flags; > > 229 #define I40EVF_FLAG_RX_CSUM_ENABLED (u32)(1) > > 230 #define I40EVF_FLAG_RX_1BUF_CAPABLE (u32)(1 << 1) > > 231 #define I40EVF_FLAG_RX_PS_CAPABLE (u32)(1 << 2) > > 232 #define I40EVF_FLAG_RX_PS_ENABLED (u32)(1 << 3) > > 233 #define I40EVF_FLAG_IN_NETPOLL (u32)(1 << 4) > > 234 #define I40EVF_FLAG_IMIR_ENABLED (u32)(1 << 5) > > 235 #define I40EVF_FLAG_MQ_CAPABLE (u32)(1 << 6) > > 236 #define I40EVF_FLAG_NEED_LINK_UPDATE (u32)(1 << 7) > > > > The flags are explicitly u32 and the ->flags member is unsigned long. > > > > And, of course, checkpatch should complain that the "volatile" is a sign > > of confusion. > > > > regards, > > dan carpenter > > Thanks, Dan, for looking at this. I'm going to fix it by changing the > flags field to u32. Right now, nothing will break because we don't use > any flag bits in the upper 32 bits. > > Can you tell me which static checker you used on this? I routinely run > the driver through sparse and coccicheck, and neither one caught this.
This was an unpublished Smatch check. I won't ever push something that warns here because unsigned long and u32 can be equivalent. regards, dan carpenter ------------------------------------------------------------------------------ CenturyLink Cloud: The Leader in Enterprise Cloud Services. Learn Why More Businesses Are Choosing CenturyLink Cloud For Critical Workloads, Development Environments & Everything In Between. Get a Quote or Start a Free Trial Today. http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk _______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
