From: Simon Wunderlich <simon.wunderlich@s2003.tu-chemnitz.de>
To: The list for a Better Approach To Mobile Ad-hoc Networking
<b.a.t.m.a.n@lists.open-mesh.org>
Subject: Re: [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags()
Date: Fri, 4 Nov 2011 16:00:36 +0100 [thread overview]
Message-ID: <20111104150035.GA12338@pandem0nium> (raw)
In-Reply-To: <20111101134348.GH20215@ritirata.org>
[-- Attachment #1: Type: text/plain, Size: 1942 bytes --]
Hey there,
On Tue, Nov 01, 2011 at 02:43:49PM +0100, Antonio Quartulli wrote:
> On Mon, Oct 31, 2011 at 01:42:01AM +0100, Simon Wunderlich wrote:
> > Hey Antonio,
> >
> > I don't quite understand what this patch is good for - it generalises the function,
> > but does not use the function at another point. So what it is good for (next to
> > add more complexity to batman ;] ).
> >
> > Furthermore it counts the changes and adds it to num_local_tt, is there a bug fix
> > hidden in this patch somewhere? :)
>
> No hidden bug-fix here :)
> This patch aims to generalise the tt_local_reset_flags() function only. As I
> discussed with Sven on IRC, this function could be quite confusing, because its
> name suggests that it can be used either for other purposes, but this is not
> true: it can only be used as it is now in the code.
>
> Therefore this patch wants to generalise it enough in order to be eventually
> used elsewhere.
OK
>
> The increment of tt_local_num is actually done inside the function and it is
> useful only for the use case proposed in the current code. To generalise the
> function I had to move the increment outside (somebody else could want to flip a
> certain flag without affecting the local counter :-) ).
Ah okay, I missed that it was already done within the function previously - sorry,
my fault. No hidden bug fix :D
>
> IMHO this patch is not strictly needed, but it would clean the code up (Until
> it doesn't add any complexity :P)
I think thats okay. I reviewed it a second time and it does what its supposed to
do as far as i can tell.
The only thing I found a little bit confusing was the variable name "new_value",
as it turns out it is only a switch (0 - switch all flags from the flags variable
off, 1 - switch these flags on). But maybe thats just me. ;)
Otherwise,
Acked-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Cheers,
Simon
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
next prev parent reply other threads:[~2011-11-04 15:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-30 19:09 [B.A.T.M.A.N.] [PATCH] batman-adv: generalise tt_local_reset_flags() Antonio Quartulli
2011-10-31 0:42 ` Simon Wunderlich
2011-11-01 13:43 ` Antonio Quartulli
2011-11-04 15:00 ` Simon Wunderlich [this message]
2011-11-07 15:47 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
2011-11-08 17:02 ` Marek Lindner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20111104150035.GA12338@pandem0nium \
--to=simon.wunderlich@s2003.tu-chemnitz.de \
--cc=b.a.t.m.a.n@lists.open-mesh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.