Code review comment for lp:~schuster/mysql-proxy/remove_unix_socket

Am 19.04.2010 um 08:48 schrieb Michael Schuster:

> Michael Schuster has proposed merging lp:~schuster/mysql-proxy/remove_unix_socket into lp:mysql-proxy.
>
> Requested reviews:
> MySQL Proxy Developers (mysql-proxy-developers)
>
> --
> https://code.launchpad.net/~schuster/mysql-proxy/remove_unix_socket/+merge/23652
> Your team MySQL Proxy Developers is subscribed to branch lp:mysql-proxy.
> === modified file 'src/network-address.c'
> --- src/network-address.c 2010-04-07 12:44:32 +0000
> +++ src/network-address.c 2010-04-19 06:48:19 +0000
> @@ -45,6 +45,8 @@
> #include <string.h>
> #include <errno.h>
> #include <fcntl.h>
> +#include <glib.h>
> +#include <glib/gstdio.h>
>
> #include "network-address.h"
> #include "glib-ext.h"
> @@ -63,10 +65,33 @@
> }
>
> void network_address_free(network_address *addr) {
> +
> if (!addr) return;
>
> +#ifndef WIN32
> + /*
> + * if the name we're freeing starts with a '/', we're
> + * looking at a unix socket which needs to be removed
> + */
> + if (addr->fail_errno == 0 && addr->name != NULL &&
> + addr->name->str != NULL) {
> + gchar *name;
> + int ret;
> +
> + name = addr->name->str;
> + if (name[0] == '/' && g_access(name, 0) == 0) {
> + ret = g_remove(name);

Only try to remove it if we can to hide the error-msg (aka noise) for it ?

> +
> + if (ret == 0)
> + g_debug("%s removing socket %s successful",
> + G_STRLOC, name);
> + else
> + g_debug("%s removing socket %s failed: %s (%d)",
> + G_STRLOC, name, strerror(errno));

... in that case the g_debug() is perhaps better a g_critical() ?

> + }
> + }
> +#endif /* WIN32 */
> +
> g_string_free(addr->name, TRUE);
> -
> g_free(addr);
> }
>
>
> === modified file 'src/network-address.h'
> --- src/network-address.h 2010-04-06 14:26:51 +0000
> +++ src/network-address.h 2010-04-19 06:48:19 +0000
> @@ -63,8 +63,8 @@
> } addr;
>
> GString *name;
> -
> network_socklen_t len;
> + int fail_errno; /* != 0: don't remove ... I guess */
> } network_address;
>
> NETWORK_API network_address *network_address_new(void);
>
> === modified file 'src/network-socket.c'
> --- src/network-socket.c 2010-04-06 14:26:51 +0000
> +++ src/network-socket.c 2010-04-19 06:48:19 +0000
> @@ -397,6 +397,7 @@
> G_STRLOC,
> con->dst->name->str,
> g_strerror(errno), errno);
> + con->dst->fail_errno = errno; /* if a unix socket, this tells us "don't remove" */
> return NETWORK_SOCKET_ERROR;
> }
>
>