[Openvpn-devel] get rid of TAG_FILE_INLINE constant

Message ID 20200508212356.18522-1-a@unstable.cc
State Accepted
Headers show
Series [Openvpn-devel] get rid of TAG_FILE_INLINE constant | expand

Commit Message

Antonio Quartulli May 8, 2020, 11:23 a.m. UTC
Now that the whole inline logic has been converted to using bool flags,
the TAG_FILE_INLINE constant is not useful anymore.

Get rid of the constant as it's now unused and to prevent any future
developer from mistakenly use it again.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---

to be applied after all other fixes, as they remove the few last usages
of this constant.


 src/openvpn/common.h | 6 ------
 src/openvpn/crypto.c | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

Comments

Antonio Quartulli May 8, 2020, 11:25 a.m. UTC | #1
I managed to totally revert the name of the constant in the commit
subject and commit message..

may somebody fix this upon merge? :-)

It should be: INLINE_FILE_TAG

Cheers,

On 08/05/2020 23:23, Antonio Quartulli wrote:
> Now that the whole inline logic has been converted to using bool flags,
> the TAG_FILE_INLINE constant is not useful anymore.
> 
> Get rid of the constant as it's now unused and to prevent any future
> developer from mistakenly use it again.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> to be applied after all other fixes, as they remove the few last usages
> of this constant.
> 
> 
>  src/openvpn/common.h | 6 ------
>  src/openvpn/crypto.c | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/src/openvpn/common.h b/src/openvpn/common.h
> index 4e6f4809..623b3e0d 100644
> --- a/src/openvpn/common.h
> +++ b/src/openvpn/common.h
> @@ -88,12 +88,6 @@ typedef unsigned long ptr_type;
>   */
>  #define PUSH_REQUEST_INTERVAL 5
>  
> -/*
> - * A sort of pseudo-filename for data provided inline within
> - * the configuration file.
> - */
> -#define INLINE_FILE_TAG "[[INLINE]]"
> -
>  /*
>   * Script security warning
>   */
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index 672aa14a..f1a52d8c 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -1189,7 +1189,7 @@ print_key_filename(const char *str, bool is_inline)
>  {
>      if (is_inline)
>      {
> -        return INLINE_FILE_TAG;
> +        return "[[INLINE]]";
>      }
>  
>      return np(str);
>
David Sommerseth May 10, 2020, 3:53 a.m. UTC | #2
On 08/05/2020 23:23, Antonio Quartulli wrote:
> Now that the whole inline logic has been converted to using bool flags,
> the TAG_FILE_INLINE constant is not useful anymore.
> 
> Get rid of the constant as it's now unused and to prevent any future
> developer from mistakenly use it again.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> to be applied after all other fixes, as they remove the few last usages
> of this constant.
> 
> 
>  src/openvpn/common.h | 6 ------
>  src/openvpn/crypto.c | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/src/openvpn/common.h b/src/openvpn/common.h
> index 4e6f4809..623b3e0d 100644
> --- a/src/openvpn/common.h
> +++ b/src/openvpn/common.h
> @@ -88,12 +88,6 @@ typedef unsigned long ptr_type;
>   */
>  #define PUSH_REQUEST_INTERVAL 5
>  
> -/*
> - * A sort of pseudo-filename for data provided inline within
> - * the configuration file.
> - */
> -#define INLINE_FILE_TAG "[[INLINE]]"
> -
>  /*
>   * Script security warning
>   */
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index 672aa14a..f1a52d8c 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -1189,7 +1189,7 @@ print_key_filename(const char *str, bool is_inline)
>  {
>      if (is_inline)
>      {
> -        return INLINE_FILE_TAG;
> +        return "[[INLINE]]";
>      }
>  
>      return np(str);
> 

This looks promising, but is not complete - and breaking compilation:


$ git grep INLINE_FILE_TAG
src/openvpn/tls_crypt.c:        client_filename = INLINE_FILE_TAG;

$ make -j5
[...]
make[3]: Entering directory `/home/davids/devel/OpenVPN/openvpn/src/openvpn'
  CC       tls_crypt.o
tls_crypt.c: In function ‘tls_crypt_v2_write_client_key_file’:
tls_crypt.c:706:27: error: ‘INLINE_FILE_TAG’ undeclared (first use in this
function)
         client_filename = INLINE_FILE_TAG;
                           ^

I haven't dug into if client_filename really needs to be set to INLINE_FILE_TAG.
Antonio Quartulli May 10, 2020, 3:59 a.m. UTC | #3
Hi,

On 10/05/2020 15:53, David Sommerseth wrote:

> This looks promising, but is not complete - and breaking compilation:
> 
> 
> $ git grep INLINE_FILE_TAG
> src/openvpn/tls_crypt.c:        client_filename = INLINE_FILE_TAG;
> 
> $ make -j5
> [...]
> make[3]: Entering directory `/home/davids/devel/OpenVPN/openvpn/src/openvpn'
>   CC       tls_crypt.o
> tls_crypt.c: In function ‘tls_crypt_v2_write_client_key_file’:
> tls_crypt.c:706:27: error: ‘INLINE_FILE_TAG’ undeclared (first use in this
> function)
>          client_filename = INLINE_FILE_TAG;
>                            ^
> 
> I haven't dug into if client_filename really needs to be set to INLINE_FILE_TAG.
> 

whoop - I prepared a patch for this last occurrence (to be applied
before this patch), but did not send it.

Incoming...
David Sommerseth May 11, 2020, 2:21 a.m. UTC | #4
On 08/05/2020 23:23, Antonio Quartulli wrote:
> Now that the whole inline logic has been converted to using bool flags,
> the TAG_FILE_INLINE constant is not useful anymore.
> 
> Get rid of the constant as it's now unused and to prevent any future
> developer from mistakenly use it again.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
> 
> to be applied after all other fixes, as they remove the few last usages
> of this constant.
> 
> 
>  src/openvpn/common.h | 6 ------
>  src/openvpn/crypto.c | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/src/openvpn/common.h b/src/openvpn/common.h
> index 4e6f4809..623b3e0d 100644
> --- a/src/openvpn/common.h
> +++ b/src/openvpn/common.h
> @@ -88,12 +88,6 @@ typedef unsigned long ptr_type;
>   */
>  #define PUSH_REQUEST_INTERVAL 5
>  
> -/*
> - * A sort of pseudo-filename for data provided inline within
> - * the configuration file.
> - */
> -#define INLINE_FILE_TAG "[[INLINE]]"
> -
>  /*
>   * Script security warning
>   */
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index 672aa14a..f1a52d8c 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -1189,7 +1189,7 @@ print_key_filename(const char *str, bool is_inline)
>  {
>      if (is_inline)
>      {
> -        return INLINE_FILE_TAG;
> +        return "[[INLINE]]";
>      }
>  
>      return np(str);
> 

So with Antonio's explaination in message-id
<6a62b6d2-7870-124f-5b5f-be657ffeb34a@unstable.cc> and applying message-id
<20200510140017.16837-1-a@unstable.cc> ([PATCH] tls-crypt-v2: fix testing of
inline key, which I just gave an ACK) before this patch, it is all good.


Acked-by: David Sommerseth <davids@openvpn.net>
Gert Doering May 11, 2020, 2:38 a.m. UTC | #5
Your patch has been applied to the master branch.  I have dutifully 
fixed the "INLINE_FILE_TAG" confusion in the commit message :-)

Lightly compile-tested.  Buildbot can do more testing (but this is one
of the patches that would break compilation anywhere, so "trivially
correct).

commit 27ad978fd6721f05f0c484a1bdbf775b0ab36ab2
Author: Antonio Quartulli
Date:   Fri May 8 23:23:56 2020 +0200

     get rid of INLINE_FILE_TAG constant

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: David Sommerseth <davids@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20200508212356.18522-1-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19863.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/common.h b/src/openvpn/common.h
index 4e6f4809..623b3e0d 100644
--- a/src/openvpn/common.h
+++ b/src/openvpn/common.h
@@ -88,12 +88,6 @@  typedef unsigned long ptr_type;
  */
 #define PUSH_REQUEST_INTERVAL 5
 
-/*
- * A sort of pseudo-filename for data provided inline within
- * the configuration file.
- */
-#define INLINE_FILE_TAG "[[INLINE]]"
-
 /*
  * Script security warning
  */
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 672aa14a..f1a52d8c 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1189,7 +1189,7 @@  print_key_filename(const char *str, bool is_inline)
 {
     if (is_inline)
     {
-        return INLINE_FILE_TAG;
+        return "[[INLINE]]";
     }
 
     return np(str);