Hi, edward


在 2021/9/30 8:55, Edward Cree 写道:

On 29/09/2021 16:50, Jian Shen wrote:

quoted

This patchset try to solve it by change the prototype of
netdev_features_t from u64 to bitmap. With this change,
it's necessary to introduce a set of bitmap operation helpers
for netdev features. Meanwhile, the functions which use
netdev_features_t as return value are also need to be changed,
return the result as an output parameter.

This might be a terrible idea, but could you not do something like
     typedef struct {
         DECLARE_BITMAP(bits, NETDEV_FEATURE_COUNT);
     } netdev_features_t;
  thereby allowing functions to carry on returning it directly?
The compiler would still likely turn it into an output parameter
  at an ABI level (at least once NETDEV_FEATURE_COUNT goes above
  64), but the amount of code churn might be significantly reduced.
Another advantage is that, whereas bitwise ops (&, |, ^) on a
  pointer (such as unsigned long *) are legal (meaning something
  like "if (features & NETIF_F_GSO_MASK)" may still compile, at
  best with a warning, despite having nonsensical semantics), they
  aren't possible on a struct; so there's less risk of unpatched
  code (perhaps merged in from another subsystem, or in out-of-tree
  modules) silently breaking — instead, any mix of new and old code
  will be caught at build time.

WDYT?
-ed
.

Thanks for your advice.  It looks good to me.

The risk of misusing bitwise ops, and not be reported by  compiler is 
truely
exist in my patchset.  I will consider the way of using structure.

shenjian