On Sun, Oct 10, 2010 at 06:39:31AM +0200, Linus L??ssing wrote:
Especially for this commit, I'm not quite sure, if the usage of the macros is ok and appropritate or just a too extensive use / abuse of them. So comments highly appreciated :).
Just my personal preference. I suggest others comment as well.
+/* Use this, if you are going to turn a [name]_enabled in bat_priv on or off */ +#define BAT_ATTR_SWITCH(_name, _mode, _post_func) \
I would probably break this up into 3 macros. (And problem rename to BOOL instead of SWITCH.) BAT_ATTR_STORE_SWITCH() BAT_ATTR_SHOW_SWITCH() BAT_ATTR_SWITCH()
where BAT_ATTR_SWITCH() just invokes the other two.
+static ssize_t store_##_name(struct kobject *kobj, struct attribute *attr, \
char *buff, size_t count) \+{ \
- struct device *dev = to_dev(kobj->parent); \
- struct net_device *net_dev = to_net_dev(dev); \
- struct bat_priv *bat_priv = netdev_priv(net_dev); \
- void (*post_func)(struct net_device *) = _post_func; \
- int ret; \
- ret = store_switch_attr(buff, count, net_dev, (char *)attr->name, \
&bat_priv->_name##_enabled); \- if (post_func && ret) \
post_func(net_dev); \- return ret; \
+} \
Maybe turn the above into an inline function and then have the macro define a tiny wrapper function which just calls the inline function?
+static ssize_t show_##_name(struct kobject *kobj, struct attribute *attr, \
char *buff) \+{ \
- struct device *dev = to_dev(kobj->parent); \
- struct bat_priv *bat_priv = netdev_priv(to_net_dev(dev)); \
- return sprintf(buff, "%s\n", \
atomic_read(&bat_priv->_name##_enabled) == 0 ? \"disabled" : "enabled"); \+}
This is small enough that it is O.K. as a macro.
Then the same again for BAT_ATTR_UINT...
Andrew