Re: [Nbd] [PATCH 1/3] nbd-server: fix open_modern() to report errors

[Wouter Verhelst <w@...112...>]

Hi Tuomas,

Sorry 'bout the delay; I didn't have much time last week, and then forgot.

I've applied all three, thanks.

On Sat, Jan 12, 2013 at 11:23:55PM +0200, Tuomas Räsänen wrote:
> This commit is part of the refactoring work on internal error
> management. The goal of the work is to separate error reporting from
> error handling to make it possible to call functions from different code
> paths with varying needs for error handling.
> 
> Another goal is to unify the error management throughout the code base.
> 
> Signed-off-by: Tuomas Räsänen <tuomasjjrasanen@...1261...>
> ---
>  nbd-server.c |   63 +++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 49 insertions(+), 14 deletions(-)
> 
> diff --git a/nbd-server.c b/nbd-server.c
> index 3d1266c..b75e290 100644
> --- a/nbd-server.c
> +++ b/nbd-server.c
> @@ -640,6 +640,10 @@ typedef enum {
>          SETUP_ERROR_SO_LINGER,               /**< Failed to set SO_LINGER to a socket */
>          SETUP_ERROR_SO_REUSEADDR,            /**< Failed to set SO_REUSEADDR to a socket */
>          SETUP_ERROR_SO_KEEPALIVE,            /**< Failed to set SO_KEEPALIVE to a socket */
> +        SETUP_ERROR_GAI,                     /**< Failed to get address info */
> +        SETUP_ERROR_SOCKET,                  /**< Failed to create a socket */
> +        SETUP_ERROR_BIND,                    /**< Failed to bind an address to socket */
> +        SETUP_ERROR_LISTEN,                  /**< Failed to start listening on a socket */
>  } SETUP_ERRORS;
>  
>  /**
> @@ -2461,12 +2465,13 @@ int setup_serve(SERVER *serve) {
>  	}
>  }
>  
> -void open_modern(const gchar *const addr, const gchar *const port) {
> +int open_modern(const gchar *const addr, const gchar *const port,
> +                GError **const gerror) {
>  	struct addrinfo hints;
>  	struct addrinfo* ai = NULL;
>  	struct sock_flags;
>  	int e;
> -        GError *gerror = NULL;
> +        int retval = -1;
>  
>  	memset(&hints, '\0', sizeof(hints));
>  	hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
> @@ -2475,28 +2480,52 @@ void open_modern(const gchar *const addr, const gchar *const port) {
>  	hints.ai_protocol = IPPROTO_TCP;
>  	e = getaddrinfo(addr, port ? port : NBD_DEFAULT_PORT, &hints, &ai);
>  	if(e != 0) {
> -		fprintf(stderr, "getaddrinfo failed: %s\n", gai_strerror(e));
> -		exit(EXIT_FAILURE);
> +                g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_GAI,
> +                            "failed to open a modern socket: "
> +                            "failed to get address info: %s",
> +                            gai_strerror(e));
> +                goto out;
>  	}
> +
>  	if((modernsock = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol))<0) {
> -		err("socket: %m");
> +                g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_SOCKET,
> +                            "failed to open a modern socket: "
> +                            "failed to create a socket: %s",
> +                            strerror(errno));
> +                goto out;
>  	}
>  
> -	if (dosockopts(modernsock, &gerror) == -1) {
> -                msg(LOG_ERR, "failed to open service-wide socket: %s",
> -                    gerror->message);
> -                g_clear_error(&gerror);
> -                exit(EXIT_FAILURE);
> +	if (dosockopts(modernsock, gerror) == -1) {
> +                g_prefix_error(gerror, "failed to open a modern socket: ");
> +                goto out;
>          }
>  
>  	if(bind(modernsock, ai->ai_addr, ai->ai_addrlen)) {
> -		err("bind: %m");
> +                g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_BIND,
> +                            "failed to open a modern socket: "
> +                            "failed to bind an address to a socket: %s",
> +                            strerror(errno));
> +                goto out;
>  	}
> +
>  	if(listen(modernsock, 10) <0) {
> -		err("listen: %m");
> +                g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_BIND,
> +                            "failed to open a modern socket: "
> +                            "failed to start listening on a socket: %s",
> +                            strerror(errno));
> +                goto out;
>  	}
>  
> -	freeaddrinfo(ai);
> +        retval = 0;
> +out:
> +
> +        if (retval == -1 && modernsock >= 0) {
> +                close(modernsock);
> +                modernsock = -1;
> +        }
> +        freeaddrinfo(ai);
> +
> +        return retval;
>  }
>  
>  /**
> @@ -2512,7 +2541,13 @@ void setup_servers(GArray *const servers, const gchar *const modernaddr,
>  		want_modern |= setup_serve(&(g_array_index(servers, SERVER, i)));
>  	}
>  	if(want_modern) {
> -		open_modern(modernaddr, modernport);
> +                GError *gerror = NULL;
> +                if (open_modern(modernaddr, modernport, &gerror) == -1) {
> +                        msg(LOG_ERR, "failed to setup servers: %s",
> +                            gerror->message);
> +                        g_clear_error(&gerror);
> +                        exit(EXIT_FAILURE);
> +                }
>  	}
>  	children=g_hash_table_new_full(g_int_hash, g_int_equal, NULL, destroy_pid_t);
>  
> -- 
> 1.7.10.4
> 
> 
> ------------------------------------------------------------------------------
> Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
> MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
> with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
> MVPs and experts. SALE $99.99 this month only -- learn more at:
> http://p.sf.net/sfu/learnmore_122912
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general

-- 
Copyshops should do vouchers. So that next time some bureaucracy requires you
to mail a form in triplicate, you can mail it just once, add a voucher, and
save on postage.