Message ID | 20161027190150.7880-18-sw@simonwunderlich.de |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2016-10-27 at 21:01 +0200, Simon Wunderlich wrote: > From: Sven Eckelmann <sven@narfation.org> > > It must be avoided that arguments to a macro are evaluated ungrouped (which > enforces normal operator precendence). Otherwise the result of the macro > is not well defined. Curiosity: in net/batman-adv/tp_meter.c static int batadv_tp_send(void *arg) { struct batadv_tp_vars *tp_vars = arg; struct batadv_priv *bat_priv = tp_vars->bat_priv; struct batadv_hard_iface *primary_if = NULL; struct batadv_orig_node *orig_node = NULL; size_t payload_len, packet_len; int err = 0; if (unlikely(tp_vars->role != BATADV_TP_SENDER)) { err = BATADV_TP_REASON_DST_UNREACHABLE; tp_vars->reason = err; goto out; } orig_node = batadv_orig_hash_find(bat_priv, tp_vars->other_end); if (unlikely(!orig_node)) { err = BATADV_TP_REASON_DST_UNREACHABLE; tp_vars->reason = err; goto out; } primary_if = batadv_primary_if_get_selected(bat_priv); if (unlikely(!primary_if)) { err = BATADV_TP_REASON_DST_UNREACHABLE; goto out; } err is not used in the out block Is the last if block supposed to set tp_vars->reason to err?
On Freitag, 28. Oktober 2016 14:13:06 CEST Joe Perches wrote: > On Thu, 2016-10-27 at 21:01 +0200, Simon Wunderlich wrote: > > From: Sven Eckelmann <sven@narfation.org> > > > > It must be avoided that arguments to a macro are evaluated ungrouped (which > > enforces normal operator precendence). Otherwise the result of the macro > > is not well defined. > > Curiosity: > > in net/batman-adv/tp_meter.c [...] > orig_node = batadv_orig_hash_find(bat_priv, tp_vars->other_end); > if (unlikely(!orig_node)) { > err = BATADV_TP_REASON_DST_UNREACHABLE; > tp_vars->reason = err; > goto out; > } > > primary_if = batadv_primary_if_get_selected(bat_priv); > if (unlikely(!primary_if)) { > err = BATADV_TP_REASON_DST_UNREACHABLE; > goto out; > } > > err is not used in the out block > > Is the last if block supposed to set tp_vars->reason to err? This seems to be unrelated to this patch. But yes, looks to me like it is missing. Do you want to propose a patch or should I do? Just make sure you Cc Antonio Quartulli <a@unstable.cc> (and of course b.a.t.m.a.n@lists.open-mesh.org). He is the original author of 33a3bb4a3345 ("batman-adv: throughput meter implementation"). Kind regards, Sven
On Fri, 2016-10-28 at 23:27 +0200, Sven Eckelmann wrote: > On Freitag, 28. Oktober 2016 14:13:06 CEST Joe Perches wrote: > > On Thu, 2016-10-27 at 21:01 +0200, Simon Wunderlich wrote: > > > From: Sven Eckelmann <sven@narfation.org> > > > > > > It must be avoided that arguments to a macro are evaluated ungrouped (which > > > enforces normal operator precendence). Otherwise the result of the macro > > > is not well defined. > > > > Curiosity: > > > > in net/batman-adv/tp_meter.c > > [...] > > orig_node = batadv_orig_hash_find(bat_priv, tp_vars->other_end); > > if (unlikely(!orig_node)) { > > err = BATADV_TP_REASON_DST_UNREACHABLE; > > tp_vars->reason = err; > > goto out; > > } > > > > primary_if = batadv_primary_if_get_selected(bat_priv); > > if (unlikely(!primary_if)) { > > err = BATADV_TP_REASON_DST_UNREACHABLE; > > goto out; > > } > > > > err is not used in the out block > > > > Is the last if block supposed to set tp_vars->reason to err? > > This seems to be unrelated to this patch. Kinda. I was looking at how the one instance of batadv_tp_is_error was used and it's in this file. > But yes, looks to me like it is missing. Do you want to propose a patch or > should I do? As you are working on this file, perhaps you should. cheers, Joe
On Freitag, 28. Oktober 2016 18:56:24 CEST Joe Perches wrote: [...] > > But yes, looks to me like it is missing. Do you want to propose a patch or > > should I do? > > As you are working on this file, perhaps you should. Ok, I will submit a patch with you as Reported-by to the batman-adv mailing list Kind regards, Sven
diff --git a/net/batman-adv/log.h b/net/batman-adv/log.h index e0e1a88..ab47acf 100644 --- a/net/batman-adv/log.h +++ b/net/batman-adv/log.h @@ -71,12 +71,12 @@ int batadv_debug_log(struct batadv_priv *bat_priv, const char *fmt, ...) __printf(2, 3); /* possibly ratelimited debug output */ -#define _batadv_dbg(type, bat_priv, ratelimited, fmt, arg...) \ - do { \ - if (atomic_read(&bat_priv->log_level) & type && \ - (!ratelimited || net_ratelimit())) \ - batadv_debug_log(bat_priv, fmt, ## arg);\ - } \ +#define _batadv_dbg(type, bat_priv, ratelimited, fmt, arg...) \ + do { \ + if (atomic_read(&(bat_priv)->log_level) & (type) && \ + (!(ratelimited) || net_ratelimit())) \ + batadv_debug_log(bat_priv, fmt, ## arg); \ + } \ while (0) #else /* !CONFIG_BATMAN_ADV_DEBUG */ __printf(4, 5) diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h index 6a2328d..daddca9 100644 --- a/net/batman-adv/main.h +++ b/net/batman-adv/main.h @@ -199,8 +199,8 @@ struct packet_type; struct seq_file; struct sk_buff; -#define BATADV_PRINT_VID(vid) ((vid & BATADV_VLAN_HAS_TAG) ? \ - (int)(vid & VLAN_VID_MASK) : -1) +#define BATADV_PRINT_VID(vid) (((vid) & BATADV_VLAN_HAS_TAG) ? \ + (int)((vid) & VLAN_VID_MASK) : -1) extern struct list_head batadv_hardif_list; diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h index d2e9bbd..7a36bcf 100644 --- a/net/batman-adv/packet.h +++ b/net/batman-adv/packet.h @@ -21,7 +21,7 @@ #include <asm/byteorder.h> #include <linux/types.h> -#define batadv_tp_is_error(n) ((u8)n > 127 ? 1 : 0) +#define batadv_tp_is_error(n) ((u8)(n) > 127 ? 1 : 0) /** * enum batadv_packettype - types for batman-adv encapsulated packets