subject:"Re\: \[Openvpn\-devel\] \[PATCH 49\/52\] build\: move wrappers into platform module"

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

2012-03-23 Thread Gert Doering

Hi,

On Fri, Mar 09, 2012 at 10:24:30AM +0200, Samuli Seppänen wrote:
> I give this one a feature-ACK, but could somebody else take a better
> look at the actual code changes?

code ACK.

Verified that code is just moved + function names renamed, and in
a few places, parts of functions moved to a new platform_... function
that has the same code.

That was a long one... :-o

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


pgpuJNb8nITls.pgp
Description: PGP signature

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

2012-03-09 Thread Samuli Seppänen

Thanks for the clarifications! I like the idea of moving some
functionality into more general-purpose libraries whenever possible.
Those libraries could then be - at least theoretically - used by other
projects, so that the code gets more exposure and testing, as well as
taking some of the maintenance load of our shoulders.

I give this one a feature-ACK, but could somebody else take a better
look at the actual code changes?

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc
 
irc freenode net: mattock




> Well, at first I wanted to split it into its own libplatform distinct
> from libcompat.
>
> libcompat - emulation of missing library functions, drop-in replacement.
> libplatform - extensions to library functions, such as unicode or
> security additions.
>
> But then I've seen that there is too much openvpn specific logic,
> especially in log messages. So I left what I would have placed in
> libplatform in platform.c for now.
>
> After this round we should consider if we want to progress in this.
>
> One missing part is the exec wrappers which needs some work before
> moving to platform.
>
> Alon.
>
> 2012/3/8 Samuli Seppänen :
>> This probably makes sense, lots of good refactorings. That said, I'd
>> like to know how you selected what goes to platform.c?
>>
>> --
>> Samuli Seppänen
>> Community Manager
>> OpenVPN Technologies, Inc
>>
>> irc freenode net: mattock
>>
>>
>>> + Some fixups within the platform.c functions.
>>> - need to check environment set on Windows.
>>>
>>> Signed-off-by: Alon Bar-Lev 
>>> ---
>>>  src/openvpn/Makefile.am|1 +
>>>  src/openvpn/buffer.c   |2 +-
>>>  src/openvpn/crypto.c   |6 +-
>>>  src/openvpn/error.c|2 +-
>>>  src/openvpn/init.c |   18 +-
>>>  src/openvpn/manage.c   |   16 +-
>>>  src/openvpn/misc.c |  295 ++--
>>>  src/openvpn/misc.h |  106 +-
>>>  src/openvpn/mstats.c   |2 +-
>>>  src/openvpn/multi.c|2 +-
>>>  src/openvpn/openvpn.h  |4 +-
>>>  src/openvpn/openvpn.vcproj |8 +
>>>  src/openvpn/options.c  |   14 +-
>>>  src/openvpn/packet_id.c|2 +-
>>>  src/openvpn/pf.c   |6 +-
>>>  src/openvpn/platform.c |  369 
>>> 
>>>  src/openvpn/platform.h |  142 +
>>>  src/openvpn/ps.c   |2 +-
>>>  src/openvpn/ssl_openssl.c  |2 +-
>>>  src/openvpn/ssl_verify.c   |8 +-
>>>  src/openvpn/status.c   |6 +-
>>>  src/openvpn/tun.c  |   12 +-
>>>  src/openvpn/win32.c|   27 
>>>  23 files changed, 584 insertions(+), 468 deletions(-)
>>>  create mode 100644 src/openvpn/platform.c
>>>  create mode 100644 src/openvpn/platform.h
>>>
>>> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
>>> index 333eebc..6ba12b8 100644
>>> --- a/src/openvpn/Makefile.am
>>> +++ b/src/openvpn/Makefile.am
>>> @@ -58,6 +58,7 @@ openvpn_SOURCES = \
>>> mbuf.c mbuf.h \
>>> memdbg.h \
>>> misc.c misc.h \
>>> +   platform.c platform.h \
>>> console.c console.h \
>>> mroute.c mroute.h \
>>> mss.c mss.h \
>>> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
>>> index ad30223..5eee3ee 100644
>>> --- a/src/openvpn/buffer.c
>>> +++ b/src/openvpn/buffer.c
>>> @@ -1080,7 +1080,7 @@ buffer_list_advance (struct buffer_list *ol, int n)
>>>  struct buffer_list *
>>>  buffer_list_file (const char *fn, int max_line_len)
>>>  {
>>> -  FILE *fp = openvpn_fopen (fn, "r");
>>> +  FILE *fp = platform_fopen (fn, "r");
>>>struct buffer_list *bl = NULL;
>>>
>>>if (fp)
>>> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
>>> index 2e2e5d7..f811966 100644
>>> --- a/src/openvpn/crypto.c
>>> +++ b/src/openvpn/crypto.c
>>> @@ -868,7 +868,7 @@ read_key_file (struct key2 *key2, const char *file, 
>>> const unsigned int flags)
>>>  #endif
>>>  {
>>>in = alloc_buf_gc (2048, );
>>> -  fd = openvpn_open (file, O_RDONLY, 0);
>>> +  fd = platform_open (file, O_RDONLY, 0);
>>>if (fd == -1)
>>> msg (M_ERR, "Cannot open file key file '%s'", file);
>>>size = read (fd, in.data, in.capacity);
>>> @@ -1029,7 +1029,7 @@ read_passphrase_hash (const char *passphrase_file,
>>>  const int min_passphrase_size = 8;
>>>  uint8_t buf[64];
>>>  int total_size = 0;
>>> -int fd = openvpn_open (passphrase_file, O_RDONLY, 0);
>>> +int fd = platform_open (passphrase_file, O_RDONLY, 0);
>>>
>>>  if (fd == -1)
>>>msg (M_ERR, "Cannot open passphrase file: '%s'", passphrase_file);
>>> @@ -1079,7 +1079,7 @@ write_key_file (const int nkeys, const char *filename)
>>>const int bytes_per_line = 16;
>>>
>>>/* open key file */
>>> -  fd = openvpn_open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
>>> S_IWUSR);
>>> +  fd = platform_open (filename, O_CREAT | O_TRUNC | O_WRONLY,

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

2012-03-08 Thread Alon Bar-Lev

Well, at first I wanted to split it into its own libplatform distinct
from libcompat.

libcompat - emulation of missing library functions, drop-in replacement.
libplatform - extensions to library functions, such as unicode or
security additions.

But then I've seen that there is too much openvpn specific logic,
especially in log messages. So I left what I would have placed in
libplatform in platform.c for now.

After this round we should consider if we want to progress in this.

One missing part is the exec wrappers which needs some work before
moving to platform.

Alon.

2012/3/8 Samuli Seppänen :
> This probably makes sense, lots of good refactorings. That said, I'd
> like to know how you selected what goes to platform.c?
>
> --
> Samuli Seppänen
> Community Manager
> OpenVPN Technologies, Inc
>
> irc freenode net: mattock
>
>
>> + Some fixups within the platform.c functions.
>> - need to check environment set on Windows.
>>
>> Signed-off-by: Alon Bar-Lev 
>> ---
>>  src/openvpn/Makefile.am    |    1 +
>>  src/openvpn/buffer.c       |    2 +-
>>  src/openvpn/crypto.c       |    6 +-
>>  src/openvpn/error.c        |    2 +-
>>  src/openvpn/init.c         |   18 +-
>>  src/openvpn/manage.c       |   16 +-
>>  src/openvpn/misc.c         |  295 ++--
>>  src/openvpn/misc.h         |  106 +-
>>  src/openvpn/mstats.c       |    2 +-
>>  src/openvpn/multi.c        |    2 +-
>>  src/openvpn/openvpn.h      |    4 +-
>>  src/openvpn/openvpn.vcproj |    8 +
>>  src/openvpn/options.c      |   14 +-
>>  src/openvpn/packet_id.c    |    2 +-
>>  src/openvpn/pf.c           |    6 +-
>>  src/openvpn/platform.c     |  369 
>> 
>>  src/openvpn/platform.h     |  142 +
>>  src/openvpn/ps.c           |    2 +-
>>  src/openvpn/ssl_openssl.c  |    2 +-
>>  src/openvpn/ssl_verify.c   |    8 +-
>>  src/openvpn/status.c       |    6 +-
>>  src/openvpn/tun.c          |   12 +-
>>  src/openvpn/win32.c        |   27 
>>  23 files changed, 584 insertions(+), 468 deletions(-)
>>  create mode 100644 src/openvpn/platform.c
>>  create mode 100644 src/openvpn/platform.h
>>
>> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
>> index 333eebc..6ba12b8 100644
>> --- a/src/openvpn/Makefile.am
>> +++ b/src/openvpn/Makefile.am
>> @@ -58,6 +58,7 @@ openvpn_SOURCES = \
>>         mbuf.c mbuf.h \
>>         memdbg.h \
>>         misc.c misc.h \
>> +       platform.c platform.h \
>>         console.c console.h \
>>         mroute.c mroute.h \
>>         mss.c mss.h \
>> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
>> index ad30223..5eee3ee 100644
>> --- a/src/openvpn/buffer.c
>> +++ b/src/openvpn/buffer.c
>> @@ -1080,7 +1080,7 @@ buffer_list_advance (struct buffer_list *ol, int n)
>>  struct buffer_list *
>>  buffer_list_file (const char *fn, int max_line_len)
>>  {
>> -  FILE *fp = openvpn_fopen (fn, "r");
>> +  FILE *fp = platform_fopen (fn, "r");
>>    struct buffer_list *bl = NULL;
>>
>>    if (fp)
>> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
>> index 2e2e5d7..f811966 100644
>> --- a/src/openvpn/crypto.c
>> +++ b/src/openvpn/crypto.c
>> @@ -868,7 +868,7 @@ read_key_file (struct key2 *key2, const char *file, 
>> const unsigned int flags)
>>  #endif
>>      {
>>        in = alloc_buf_gc (2048, );
>> -      fd = openvpn_open (file, O_RDONLY, 0);
>> +      fd = platform_open (file, O_RDONLY, 0);
>>        if (fd == -1)
>>         msg (M_ERR, "Cannot open file key file '%s'", file);
>>        size = read (fd, in.data, in.capacity);
>> @@ -1029,7 +1029,7 @@ read_passphrase_hash (const char *passphrase_file,
>>      const int min_passphrase_size = 8;
>>      uint8_t buf[64];
>>      int total_size = 0;
>> -    int fd = openvpn_open (passphrase_file, O_RDONLY, 0);
>> +    int fd = platform_open (passphrase_file, O_RDONLY, 0);
>>
>>      if (fd == -1)
>>        msg (M_ERR, "Cannot open passphrase file: '%s'", passphrase_file);
>> @@ -1079,7 +1079,7 @@ write_key_file (const int nkeys, const char *filename)
>>    const int bytes_per_line = 16;
>>
>>    /* open key file */
>> -  fd = openvpn_open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
>> S_IWUSR);
>> +  fd = platform_open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
>> S_IWUSR);
>>
>>    if (fd == -1)
>>      msg (M_ERR, "Cannot open shared secret file '%s' for write", filename);
>> diff --git a/src/openvpn/error.c b/src/openvpn/error.c
>> index 1f2dd86..d6ad639 100644
>> --- a/src/openvpn/error.c
>> +++ b/src/openvpn/error.c
>> @@ -640,7 +640,7 @@ x_check_status (int status,
>>                  my_errno);
>>
>>           if (x_cs_err_delay_ms)
>> -           sleep_milliseconds (x_cs_err_delay_ms);
>> +           platform_sleep_milliseconds (x_cs_err_delay_ms);
>>         }
>>        gc_free ();
>>      }
>> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
>> index bba3cf8..bc7718e 100644
>> ---

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

2012-03-08 Thread Samuli Seppänen

This probably makes sense, lots of good refactorings. That said, I'd
like to know how you selected what goes to platform.c?

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock


> + Some fixups within the platform.c functions.
> - need to check environment set on Windows.
>
> Signed-off-by: Alon Bar-Lev 
> ---
>  src/openvpn/Makefile.am|1 +
>  src/openvpn/buffer.c   |2 +-
>  src/openvpn/crypto.c   |6 +-
>  src/openvpn/error.c|2 +-
>  src/openvpn/init.c |   18 +-
>  src/openvpn/manage.c   |   16 +-
>  src/openvpn/misc.c |  295 ++--
>  src/openvpn/misc.h |  106 +-
>  src/openvpn/mstats.c   |2 +-
>  src/openvpn/multi.c|2 +-
>  src/openvpn/openvpn.h  |4 +-
>  src/openvpn/openvpn.vcproj |8 +
>  src/openvpn/options.c  |   14 +-
>  src/openvpn/packet_id.c|2 +-
>  src/openvpn/pf.c   |6 +-
>  src/openvpn/platform.c |  369 
> 
>  src/openvpn/platform.h |  142 +
>  src/openvpn/ps.c   |2 +-
>  src/openvpn/ssl_openssl.c  |2 +-
>  src/openvpn/ssl_verify.c   |8 +-
>  src/openvpn/status.c   |6 +-
>  src/openvpn/tun.c  |   12 +-
>  src/openvpn/win32.c|   27 
>  23 files changed, 584 insertions(+), 468 deletions(-)
>  create mode 100644 src/openvpn/platform.c
>  create mode 100644 src/openvpn/platform.h
>
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index 333eebc..6ba12b8 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -58,6 +58,7 @@ openvpn_SOURCES = \
> mbuf.c mbuf.h \
> memdbg.h \
> misc.c misc.h \
> +   platform.c platform.h \
> console.c console.h \
> mroute.c mroute.h \
> mss.c mss.h \
> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
> index ad30223..5eee3ee 100644
> --- a/src/openvpn/buffer.c
> +++ b/src/openvpn/buffer.c
> @@ -1080,7 +1080,7 @@ buffer_list_advance (struct buffer_list *ol, int n)
>  struct buffer_list *
>  buffer_list_file (const char *fn, int max_line_len)
>  {
> -  FILE *fp = openvpn_fopen (fn, "r");
> +  FILE *fp = platform_fopen (fn, "r");
>struct buffer_list *bl = NULL;
>
>if (fp)
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index 2e2e5d7..f811966 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -868,7 +868,7 @@ read_key_file (struct key2 *key2, const char *file, const 
> unsigned int flags)
>  #endif
>  {
>in = alloc_buf_gc (2048, );
> -  fd = openvpn_open (file, O_RDONLY, 0);
> +  fd = platform_open (file, O_RDONLY, 0);
>if (fd == -1)
> msg (M_ERR, "Cannot open file key file '%s'", file);
>size = read (fd, in.data, in.capacity);
> @@ -1029,7 +1029,7 @@ read_passphrase_hash (const char *passphrase_file,
>  const int min_passphrase_size = 8;
>  uint8_t buf[64];
>  int total_size = 0;
> -int fd = openvpn_open (passphrase_file, O_RDONLY, 0);
> +int fd = platform_open (passphrase_file, O_RDONLY, 0);
>
>  if (fd == -1)
>msg (M_ERR, "Cannot open passphrase file: '%s'", passphrase_file);
> @@ -1079,7 +1079,7 @@ write_key_file (const int nkeys, const char *filename)
>const int bytes_per_line = 16;
>
>/* open key file */
> -  fd = openvpn_open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
> S_IWUSR);
> +  fd = platform_open (filename, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | 
> S_IWUSR);
>
>if (fd == -1)
>  msg (M_ERR, "Cannot open shared secret file '%s' for write", filename);
> diff --git a/src/openvpn/error.c b/src/openvpn/error.c
> index 1f2dd86..d6ad639 100644
> --- a/src/openvpn/error.c
> +++ b/src/openvpn/error.c
> @@ -640,7 +640,7 @@ x_check_status (int status,
>  my_errno);
>
>   if (x_cs_err_delay_ms)
> -   sleep_milliseconds (x_cs_err_delay_ms);
> +   platform_sleep_milliseconds (x_cs_err_delay_ms);
> }
>gc_free ();
>  }
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index bba3cf8..bc7718e 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -935,7 +935,7 @@ do_genkey (const struct options * options)
>"shared secret output file (--secret)");
>
>if (options->mlock)  /* should we disable paging? */
> -   do_mlockall (true);
> +   platform_mlockall (true);
>
>nbits_written = write_key_file (2, options->shared_secret_file);
>
> @@ -1022,7 +1022,7 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
>if (c->options.chroot_dir)
> {
>   if (no_delay)
> -   do_chroot (c->options.chroot_dir);
> +   platform_chroot (c->options.chroot_dir);
>   else
> msg (M_INFO, "NOTE: chroot %s", why_not);
> }
> @@ -1030,8 +1030,8 @@

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module

4 matches

Site Navigation

Mail list logo

Footer information