Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module
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, &gc); > - 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 (&gc); > } > 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 @@ do_uid_gid_chroot (st
Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module
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, &gc); >> - 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 (&gc); >> } >> diff --git a/src/openvpn/init.c b/src/openvpn/init.c >> index bba3cf8..bc7718e 100644 >> --- a/src/openvpn/init.c >> +++ b/src/ope
Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module
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, &gc); >>> - 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 (
Re: [Openvpn-devel] [PATCH 49/52] build: move wrappers into platform module
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