Re: [B.A.T.M.A.N.] [PATCH 1/3] batman-adv: atomic variable for vis-srv activation

["Linus Lüssing" <linus.luessing@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

On Sun, Jan 03, 2010 at 03:26:02AM +0800, Marek Lindner wrote:
> 
> Hi,
> 
> > This fixes the bug discovered by Marek which did not allow turning on
> > the vis-server before an interface has been added. This is now being
> > done in a similar way as for (de)activating the aggregation mode with an
> > atomic variable.
> 
> great that you work on this!  :)
> 
> I'm no expert on the vis code but here my 2 cents:
> * You hard-coded integer values instead of using the existing defines. Any 
> reason for that ?
Hmm, so far we are having too modes only, vis server being enabled
or disabled. But in those packet functions we are talking about
types (two ones only so far again) instead. In the second one I found it
ok to use the defines, but for the proc.c switching, I found a
simple 0 or 1 more intuitive, as you already know that this
function is for enabling/disabling the vis server and you don't
have to wonder about a certain vis-types field in the vis-packet
format. Hmm, I'm also wondering, are there any plans for
introducing other vis_types? (What would you think about changing
the 8 bits vis_type field to flags instead?)
> * my_vis_info->packet.vis_type never gets updated ?
Damn it, you are right :). I assume, that it'd be ok to remove the default
value being set in vis_init and explicitly setting this field
every time a new vis packet gets generated in generate_vis_packet()
(instead of updating this variable from proc.c too)?
> * This patch removes functionality from the /proc parsing function although 
> this is not related to the patch-topic.
Yes, sorry, you're right I'd removed the
server/client/enabled/disabled things and added just 0/1 as input
values for proc to make the syntax similar to the aggregation
stuff and simple in general for the kernel module. I'm going to put
that in a seperate patch.
> * checkpatch reports 8 errors & 8 warnings
Ehm, just got 1 error at the else part... (sorry, never used
checkpatch before, hope that I'm not handling it wrong in any
way...)
> 
> Regards,
> Marek
> _______________________________________________
> B.A.T.M.A.N mailing list
> B.A.T.M.A.N@lists.open-mesh.net
> https://lists.open-mesh.net/mm/listinfo/b.a.t.m.a.n
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]