Re: [Openvpn-devel] [PATCH 1/2] merge *-inline.h files with their main header

[Antonio Quartulli]

On 18/09/17 13:18, Antonio Quartulli wrote:
> Hi,
> 
> On 17/09/17 20:00, Steffan Karger wrote:
> 
> [CUT]
> 
>>> @@ -101,4 +104,16 @@ void pf_context_print(const struct pf_context *pfc, 
>>> const char *prefix, const in
>>>  
>>>  #endif
>>>  
>>> +bool pf_c2c_test(const struct context *src, const struct context *dest,
>>> + const char *prefix);
>>> +
>>> +bool pf_addr_test(const struct context *src, const struct mroute_addr 
>>> *dest,
>>> +  const char *prefix);
>>
>> Why are these no longer inline?  They seem to be simple wrappers to
>> prevent a function call is pf is not enabled.
>>
> 
> Oh, I needed sometime, but then I understood what you mean :)
> 
> You are saying that by keeping this function inline, we have a slight
> performance gain because we won't invoke pf_cn_test() when pf is
> disabled. And such decision can be made inline.

Now I recall why I changed that.

pf_c2c_test is dereferencing an object of type "struct context" for
which we have no definition available at that point.

A fix could be to include openvpn.h in pf.h, but then we get other weird
problems due to circular inclusions. Having pf-inline.h was, of course,
much easier because you could place its inclusion at the most convenient
place and the problem was solved.

I'll try to restructure the functions a bit so that we don't lose the
gain given by the inline attribute, but we don't need to dereference the
context as well.

Cheers,

> 
> 
> I agree. I will send a v2 of this patch.
> 
> 
>>>
>>
>> Otherwise this patch looks good.
> 
> Thanks for reviewing!
> 
> Cheers,
> 
> 
> 
> 
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> 
> 
> 
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
> 

-- 
Antonio Quartulli



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/2] merge *-inline.h files with their main header

[Steffan Karger]

Hi,

On 28-08-17 10:56, Antonio Quartulli wrote:
> *-inline.h files are not very useful anymore.
> In the attempt of cleaning up the code some more,
> merge them into their main header files.
> 
> No functional change is part of this patch.
> 
> Signed-off-by: Antonio Quartulli 
> ---
> 
> patch has been verified with travis-ci
> 
>  src/openvpn/Makefile.am |   8 +-
>  src/openvpn/forward-inline.h| 341 
> 
>  src/openvpn/forward.c   |   5 +-
>  src/openvpn/forward.h   | 319 -
>  src/openvpn/init.c  |   4 +-
>  src/openvpn/mtcp.c  |   2 +-
>  src/openvpn/mudp.c  |   2 +-
>  src/openvpn/multi.c |   4 +-
>  src/openvpn/occ-inline.h|  95 --
>  src/openvpn/occ.c   |   4 +-
>  src/openvpn/occ.h   |  61 +++
>  src/openvpn/openvpn.c   |   2 -
>  src/openvpn/openvpn.vcxproj |   4 -
>  src/openvpn/openvpn.vcxproj.filters |  12 --
>  src/openvpn/pf-inline.h |  63 ---
>  src/openvpn/pf.c|  24 ++-
>  src/openvpn/pf.h|  15 ++
>  src/openvpn/ping-inline.h   |  64 ---
>  src/openvpn/ping.c  |   1 -
>  src/openvpn/ping.h  |  37 
>  20 files changed, 465 insertions(+), 602 deletions(-)
>  delete mode 100644 src/openvpn/forward-inline.h
>  delete mode 100644 src/openvpn/occ-inline.h
>  delete mode 100644 src/openvpn/pf-inline.h
>  delete mode 100644 src/openvpn/ping-inline.h
> 
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index fcc22d68..babc0adb 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -55,7 +55,7 @@ openvpn_SOURCES = \
>   error.c error.h \
>   event.c event.h \
>   fdmisc.c fdmisc.h \
> - forward.c forward.h forward-inline.h \
> + forward.c forward.h \
>   fragment.c fragment.h \
>   gremlin.c gremlin.h \
>   helper.c helper.h \
> @@ -80,7 +80,7 @@ openvpn_SOURCES = \
>   mudp.c mudp.h \
>   multi.c multi.h \
>   ntlm.c ntlm.h \
> - occ.c occ.h occ-inline.h \
> + occ.c occ.h \
>   openssl_compat.h \
>   pkcs11.c pkcs11.h pkcs11_backend.h \
>   pkcs11_openssl.c \
> @@ -90,8 +90,8 @@ openvpn_SOURCES = \
>   otime.c otime.h \
>   packet_id.c packet_id.h \
>   perf.c perf.h \
> - pf.c pf.h pf-inline.h \
> - ping.c ping.h ping-inline.h \
> + pf.c pf.h \
> + ping.c ping.h \
>   plugin.c plugin.h \
>   pool.c pool.h \
>   proto.c proto.h \
> diff --git a/src/openvpn/forward-inline.h b/src/openvpn/forward-inline.h
> deleted file mode 100644
> index ab83ea40..
> --- a/src/openvpn/forward-inline.h
> +++ /dev/null
> @@ -1,341 +0,0 @@
> -/*
> - *  OpenVPN -- An application to securely tunnel IP networks
> - * over a single TCP/UDP port, with support for SSL/TLS-based
> - * session authentication and key exchange,
> - * packet encryption, packet authentication, and
> - * packet compression.
> - *
> - *  Copyright (C) 2002-2017 OpenVPN Technologies, Inc. 
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License version 2
> - *  as published by the Free Software Foundation.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License along
> - *  with this program; if not, write to the Free Software Foundation, Inc.,
> - *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> - */
> -
> -#ifndef FORWARD_INLINE_H
> -#define FORWARD_INLINE_H
> -
> -/*
> - * Inline functions
> - */
> -
> -/*
> - * Does TLS session need service?
> - */
> -static inline void
> -check_tls(struct context *c)
> -{
> -#if defined(ENABLE_CRYPTO)
> -void check_tls_dowork(struct context *c);
> -
> -if (c->c2.tls_multi)
> -{
> -check_tls_dowork(c);
> -}
> -#endif
> -}
> -
> -/*
> - * TLS errors are fatal in TCP mode.
> - * Also check for --tls-exit trigger.
> - */
> -static inline void
> -check_tls_errors(struct context *c)
> -{
> -#if defined(ENABLE_CRYPTO)
> -void check_tls_errors_co(struct context *c);
> -
> -void check_tls_errors_nco(struct context *c);
> -
> -if (c->c2.tls_multi && c->c2.tls_exit_signal)
> -{
> -if (link_socket_connection_oriented(c->c2.link_socket))
> -{
> -if (c->c2.tls_multi->n_soft_errors)
> -{
> -check_tls_errors_co(c);
> -}
> -}
> -else
> -