On Tue, Nov 3, 2015 at 11:03 AM, Nikolay Aleksandrov <niko...@cumulusnetworks.com> wrote: > On 11/03/2015 03:55 AM, Jarod Wilson wrote: > [snip] >> >> +#define for_each_netdev_feature(mask_addr, feature) >> \ >> + int bit; >> \ >> + for_each_set_bit(bit, (unsigned long *)mask_addr, >> NETDEV_FEATURE_COUNT) \ >> + feature = __NETIF_F_BIT(bit); >> + > ^ > This is broken, it will not work for more than a single feature.
Indeed it is. This is used as: for_each_netdev_feature(&upper_disables, feature) { ... } which expands to: int bit; for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT) feature = __NETIF_F_BIT(bit); { ... } Note the assignment to "feature" happens outside the {}-delimited block. And the block is always executed once. Interestingly, arm-unknown-linux-gnueabi-gcc 4.9.0 warns about this in some of my configs, but not in all of them: net/core/dev.c: In function '__netdev_update_features': net/core/dev.c:6302:16: warning: 'feature' may be used uninitialized in this function [-Wmaybe-uninitialized] features &= ~feature; ^ net/core/dev.c:6295:20: note: 'feature' was declared here netdev_features_t feature; ^ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/