Re: [B.A.T.M.A.N.] [PATCH v2 3/4] batman-adv: Introduce packet type independent TVLV handler API

[Sven Eckelmann <sven@narfation.org>]

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

On Dienstag, 20. Dezember 2016 12:55:50 CET Linus Lüssing wrote:
> On Mon, Dec 19, 2016 at 12:43:49PM +0100, Sven Eckelmann wrote:
> > On Montag, 19. Dezember 2016 12:37:43 CET Sven Eckelmann wrote:
> > > I was not talking about the packing functions.
> > 
> > Just read my initial mails. Looks like I've copied the wrong function name 
> > "batadv_tvlv_ogm_pack_ctx". But I was talking about the parameter in of 
> > "batadv_tvlv_containers_process2". So I am the reason for the confusion :)
> 
> Just to check, you suggest removing this line:
> https://git.open-mesh.org/batman-adv.git/blob/1ddd189528fc332bf286ffd56b629da26a6b149c:/net/batman-adv/tvlv.c#l662

Yes

> And to replace the passing of "ctx" with an "orig_node" here:
> https://git.open-mesh.org/batman-adv.git/blob/1ddd189528fc332bf286ffd56b629da26a6b149c:/net/batman-adv/tvlv.c#l682

Yes

> (which works as a "void *" parameter accepts any type, without the
> need of casting explicitly)

Yes

> If that's what you mean, hm, not sure. Feels dangerous, such
> automatic casting, doesn't it?

It is not perfect but it is the way it is already done for many things in the
kernel and also in userspace. Think for example about "the argument" when you
generate (k)threads and which you can then then retrieve again inside this
thread.

Your current extra functions also don't prevent that there is a wrong argument
added to this perticular function. They just introduce some weird extra
functions. (At least this is how feel about it - their might be different
opinions about that)

> 
> I'm really wondering whether I should go for the approach of
> hiding the "void *" in a new type. And only having the "void *"
> fiddeling inside _pack()/_unpack() functions.

Hm, I personally don't see a reason for a new type at the moment. But when you
find a good one... at least their could be plenty of reasons which might make
everything a little bit cleaner.

Just keep in mind that there are already many other things in the kernel like
container_of and similar things. So don't try to overengineer it :)

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]