[17/17] batman-adv: Avoid precedence issues in macros
Commit Message
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.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/log.h | 12 ++++++------
net/batman-adv/main.h | 4 ++--
net/batman-adv/packet.h | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
Comments
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
@@ -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)
@@ -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;
@@ -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