[Openvpn-devel,7/7] Remove TLS_MODE

  Now that ENABLE_CRYPTO has been removed, TLS_MODE is basically
a useless shortcut which does not really help the readability of the
code.

Remove it and use its expanded expression instead.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 src/openvpn/forward.c | 2 +-
 src/openvpn/init.c    | 2 +-
 src/openvpn/occ.c     | 3 ++-
 src/openvpn/openvpn.h | 1 -
 4 files changed, 4 insertions(+), 4 deletions(-)
  

Hi,

On 02-12-17 14:45, Antonio Quartulli wrote:
> Now that ENABLE_CRYPTO has been removed, TLS_MODE is basically
> a useless shortcut which does not really help the readability of the
> code.

So I don't really agree here.  As we've just experienced while
discussing this on IRC, the meaning of 'c->c2.tls_multi != NULL' is not
that trivial.  At first sight it seems to suggest 'we have multiple
peers' (i.e. mode server), but it really does just mean 'this is a TLS
mode connection' (ie, not a static key connection).

So even though I would have preferred this to be an inline function, I
think it *does* add to readability.  So I'd prefer to either keep it, or
change it to a static inline function for type safety.

> Remove it and use its expanded expression instead.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  src/openvpn/forward.c | 2 +-
>  src/openvpn/init.c    | 2 +-
>  src/openvpn/occ.c     | 3 ++-
>  src/openvpn/openvpn.h | 1 -
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 9bf9483e..85ec71ae 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -948,7 +948,7 @@ process_incoming_link_part2(struct context *c, struct link_socket_info *lsi, con
>           *
>           * Also, update the persisted version of our packet-id.
>           */
> -        if (!TLS_MODE(c))
> +        if (!c->c2.tls_multi)
>          {
>              link_socket_set_outgoing_addr(&c->c2.buf, lsi, &c->c2.from, NULL, c->c2.es);
>          }
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index f8034ec7..7fe50628 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1306,7 +1306,7 @@ do_init_timers(struct context *c, bool deferred)
>          /* initialize occ timers */
>  
>          if (c->options.occ
> -            && !TLS_MODE(c)
> +            && !c->c2.tls_multi
>              && c->c2.options_string_local && c->c2.options_string_remote)
>          {
>              event_timeout_init(&c->c2.occ_interval, OCC_INTERVAL_SECONDS, now);
> diff --git a/src/openvpn/occ.c b/src/openvpn/occ.c
> index 40f7e768..8434920c 100644
> --- a/src/openvpn/occ.c
> +++ b/src/openvpn/occ.c
> @@ -378,7 +378,8 @@ process_received_occ_msg(struct context *c)
>  
>          case OCC_REPLY:
>              dmsg(D_PACKET_CONTENT, "RECEIVED OCC_REPLY");
> -            if (c->options.occ && !TLS_MODE(c) && c->c2.options_string_remote)
> +            if (c->options.occ && !c->c2.tls_multi
> +                && c->c2.options_string_remote)
>              {
>                  if (!options_cmp_equal_safe((char *) BPTR(&c->c2.buf),
>                                              c->c2.options_string_remote,
> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
> index fe8324ab..b36ca319 100644
> --- a/src/openvpn/openvpn.h
> +++ b/src/openvpn/openvpn.h
> @@ -556,7 +556,6 @@ struct context
>   * have been compiled in.
>   */
>  
> -#define TLS_MODE(c) ((c)->c2.tls_multi != NULL)
>  #define PROTO_DUMP_FLAGS (check_debug_level(D_LINK_RW_VERBOSE) ? (PD_SHOW_DATA|PD_VERBOSE) : 0)
>  #define PROTO_DUMP(buf, gc) protocol_dump((buf), \
>                                            PROTO_DUMP_FLAGS   \
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
  

On 03/12/17 22:45, Steffan Karger wrote:
> Hi,
> 
> On 02-12-17 14:45, Antonio Quartulli wrote:
>> Now that ENABLE_CRYPTO has been removed, TLS_MODE is basically
>> a useless shortcut which does not really help the readability of the
>> code.
> 
> So I don't really agree here.  As we've just experienced while
> discussing this on IRC, the meaning of 'c->c2.tls_multi != NULL' is not
> that trivial.  At first sight it seems to suggest 'we have multiple
> peers' (i.e. mode server), but it really does just mean 'this is a TLS
> mode connection' (ie, not a static key connection).
> 
> So even though I would have preferred this to be an inline function, I
> think it *does* add to readability.  So I'd prefer to either keep it, or
> change it to a static inline function for type safety.


Let's drop this patch then.

With another patch we can convert all these macros to inline static
functions.

Cheers,

> 
>> Remove it and use its expanded expression instead.
>>
>> Signed-off-by: Antonio Quartulli <a@unstable.cc>
>> ---
>>  src/openvpn/forward.c | 2 +-
>>  src/openvpn/init.c    | 2 +-
>>  src/openvpn/occ.c     | 3 ++-
>>  src/openvpn/openvpn.h | 1 -
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
>> index 9bf9483e..85ec71ae 100644
>> --- a/src/openvpn/forward.c
>> +++ b/src/openvpn/forward.c
>> @@ -948,7 +948,7 @@ process_incoming_link_part2(struct context *c, struct link_socket_info *lsi, con
>>           *
>>           * Also, update the persisted version of our packet-id.
>>           */
>> -        if (!TLS_MODE(c))
>> +        if (!c->c2.tls_multi)
>>          {
>>              link_socket_set_outgoing_addr(&c->c2.buf, lsi, &c->c2.from, NULL, c->c2.es);
>>          }
>> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
>> index f8034ec7..7fe50628 100644
>> --- a/src/openvpn/init.c
>> +++ b/src/openvpn/init.c
>> @@ -1306,7 +1306,7 @@ do_init_timers(struct context *c, bool deferred)
>>          /* initialize occ timers */
>>  
>>          if (c->options.occ
>> -            && !TLS_MODE(c)
>> +            && !c->c2.tls_multi
>>              && c->c2.options_string_local && c->c2.options_string_remote)
>>          {
>>              event_timeout_init(&c->c2.occ_interval, OCC_INTERVAL_SECONDS, now);
>> diff --git a/src/openvpn/occ.c b/src/openvpn/occ.c
>> index 40f7e768..8434920c 100644
>> --- a/src/openvpn/occ.c
>> +++ b/src/openvpn/occ.c
>> @@ -378,7 +378,8 @@ process_received_occ_msg(struct context *c)
>>  
>>          case OCC_REPLY:
>>              dmsg(D_PACKET_CONTENT, "RECEIVED OCC_REPLY");
>> -            if (c->options.occ && !TLS_MODE(c) && c->c2.options_string_remote)
>> +            if (c->options.occ && !c->c2.tls_multi
>> +                && c->c2.options_string_remote)
>>              {
>>                  if (!options_cmp_equal_safe((char *) BPTR(&c->c2.buf),
>>                                              c->c2.options_string_remote,
>> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
>> index fe8324ab..b36ca319 100644
>> --- a/src/openvpn/openvpn.h
>> +++ b/src/openvpn/openvpn.h
>> @@ -556,7 +556,6 @@ struct context
>>   * have been compiled in.
>>   */
>>  
>> -#define TLS_MODE(c) ((c)->c2.tls_multi != NULL)
>>  #define PROTO_DUMP_FLAGS (check_debug_level(D_LINK_RW_VERBOSE) ? (PD_SHOW_DATA|PD_VERBOSE) : 0)
>>  #define PROTO_DUMP(buf, gc) protocol_dump((buf), \
>>                                            PROTO_DUMP_FLAGS   \
>>
> 
> ------------------------------------------------------------------------------
> 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
>