On Thu, Mar 26, 2020 at 11:56:27AM +0100, Martin Pieuchot wrote:
> On 26/03/20(Thu) 13:34, Vitaliy Makkoveev wrote:
> > Add missing #ifdefs to pppx_if_destroy() as it done in
> > pipex_destroy_session(). Also remove unnecessary cast.
>
> What's the point of such #ifdef? I understand the current code is not
> coherent, but does this reduce the binary size? For a case in a switch?
> Because it clearly complicates the understanding of the code.
Code coherency is the goal.
> So maybe having #if defined(PIPEX_PPTP) || defined(PIPEX_L2TP) there as
> well as in pppx_add_session() and pipex_destroy_session() is the way to
> go.
>
> But the underlying question would it make sense to have pppx_if_destroy()
> and pipex_destroy_session() call the same function to clear sessions?
>
> The same could be add for pipex_add_session() and pppx_add_session().
My next goal.
I updated this diff with '#if defined()...' and for
pipex_destroy_session() too.
Index: sys/net/if_pppx.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
--- sys/net/if_pppx.c 20 Feb 2020 16:56:52 -0000 1.76
+++ sys/net/if_pppx.c 26 Mar 2020 11:27:07 -0000
@@ -967,13 +967,18 @@ pppx_if_destroy(struct pppx_dev *pxd, st
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
+#ifdef PIPEX_PPTP
case PIPEX_PROTO_PPTP:
+#endif
+#ifdef PIPEX_L2TP
case PIPEX_PROTO_L2TP:
- LIST_REMOVE((struct pipex_session *)session,
- peer_addr_chain);
+#endif
+ LIST_REMOVE(session, peer_addr_chain);
break;
}
+#endif
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(&pipex_session_list))
Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.108
diff -u -p -r1.108 pipex.c
--- sys/net/pipex.c 25 Mar 2020 11:39:58 -0000 1.108
+++ sys/net/pipex.c 26 Mar 2020 11:27:08 -0000
@@ -581,16 +581,19 @@ pipex_destroy_session(struct pipex_sessi
LIST_REMOVE(session, id_chain);
LIST_REMOVE(session, session_list);
+#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
+ switch (session->protocol) {
#ifdef PIPEX_PPTP
- if (session->protocol == PIPEX_PROTO_PPTP) {
- LIST_REMOVE(session, peer_addr_chain);
- }
+ case PIPEX_PROTO_PPTP:
#endif
#ifdef PIPEX_L2TP
- if (session->protocol == PIPEX_PROTO_L2TP) {
+ case PIPEX_PROTO_L2TP:
+#endif
LIST_REMOVE(session, peer_addr_chain);
+ break;
}
#endif
+
/* if final session is destroyed, stop timer */
if (LIST_EMPTY(&pipex_session_list))
pipex_timer_stop();
