[Swan-dev] Fwd: [Swan-commit] Changes to ref refs/heads/master

[Antony Antony]

On Wed, Nov 21, 2018 at 10:44:14AM -0500, Andrew Cagney wrote:
> Yea, two changes:
> 
> - move the function to demux.[hc]
> The function, while currently IKEv2 specific, belongs in demux.[hc] as
> it operates on the msg_digest; and like the commit points out, several
> strange #includes were eliminated
> 
> - rename v2_msg_role() to msg_role()
> This one I umed and arhd over a bit.  It makes code like:
>     switch (msg_role(md)) {
>     case MESSAGE_REQUEST: ...
> slightly less verbose.  However, to your point, it seems that only
> IKEv2 can easily figure out if a message is a request or a response
> (which really scares me).  Given this, should
> MESSAGE_{REQUEST,RESPONSE} also get the V2_* treatment vis:
>     switch(v2_msg_role(md)) {
>     case V2_MESSAGE_REQUEST: ...
> but that is really looking cumbersome.

I am confused about these series of changes. 3 concerns comes to mind about v2_msg_role and varients.

1. Me like ikev2_ (firs prefix) function names. Just v2 is confusing to me. It could be v2 of something else. nss.. also v2 preix appears at various positions. Some times as the first prefix, other times in the middle. Just some functions we have v2 versions. However, this has been going on for while, possibly too late to change. 

2 is_msg_request() in ikev2.c seems very close to the new msg_role?
There are too much git moving around to track them down.  I can't tell what is the intention here.

3. if msg_role() is in demux.c it should be ikev2_msg_role()

While quibbling I have another one.
Andrew is lately adding a bunch variables and function names with same name. It is annoying to read:) It makes hard to find definations, and declerations using cscope or grep. Any chance we can stop this trend? Once stopped slowely clean up the past.

e.g 
ikev2_message.h  25 struct ike_sa;
state.c         539 struct ike_sa *ike_sa(struct state *st)

-antony