Hi, On Thu, Dec 08, 2011 at 04:24:20PM +0100, David Sommerseth wrote: > This kicks out the openvpn_basename() function from misc.[ch] and puts > glibc equivalents into compat.[ch]. This is to provide the same > functionality on platforms not having a native basename() function > available. > > In addition this patch adds dirname() which commit 0f2bc0dd92f43c91e > depends. Without dirname(), openvpn won't build in Visual Studio. > > v2: Move all functions from compat.h to compat.c > v3: Use glibc versions of basename() and dirname() instead
ACK for the dirname() part.
I'm not particularily happy with the basename() changes, though.
> --- a/init.c
> +++ b/init.c
> @@ -862,8 +862,10 @@ init_verb_mute (struct context *c, unsigned int flags)
> void
> init_options_dev (struct options *options)
> {
> - if (!options->dev)
> - options->dev = openvpn_basename (options->dev_node);
> + if (!options->dev && options->dev_node) {
> + char *dev_node = strdup(options->dev_node); /* POSIX basename()
> implementaions may modify its arguments */
> + options->dev = basename (dev_node);
> + }
> }
These changes, just to take into account some system somewhere that
has a basename() implementation that modifies the argument string, are
ugly, and prone to memleak-errors. Also, some other "basename(3)" man
pages talk about basename() returning a pointer to an internal static
storage which can be overwritten at the next call - so the code above
would then not work on such a system (as the next call to basename()
would modify the memory are where options->dev is pointing to).
So I'd propose to keep openvpn_basename() - it's simple and short enough,
and has well-defined semantics that can be relied upon.
OTOH, David's compat.c basename() is nicer than what we currently have -
so let's use that implementation, but let's call it openvpn_basename().
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]
fax: +49-89-35655025 [email protected]
pgpnOp6yTD_pn.pgp
Description: PGP signature
