On Fri, Jan 11, 2013 at 11:54:21PM +0200, Tuomas Jorma Juhani Räsänen wrote:
> This commit changes setup_serve() to report its errors via GError object
> to its caller. It also add resource deallocation code to return points
> to free all local resources correctly. Note that, the previous version
> of the setup_serve() did not free its resources properly on error, but
> just called functions which terminated the process. This was techincally
> a leak, but without any practical significance due to immediate process
> termination. However, now that error handling (process termination) is
> separeted from error reporting, we must deallocate/free all locally
> allocated resources correctly on errors.
>
> Signed-off-by: Tuomas Jorma Juhani Räsänen <[email protected]>
> ---
> nbd-server.c | 73
> +++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/nbd-server.c b/nbd-server.c
> index 87f63fc..be65ebb 100644
> --- a/nbd-server.c
> +++ b/nbd-server.c
> @@ -2392,12 +2392,11 @@ int dosockopts(const int socket, GError **const
> gerror) {
> *
> * @param serve the server we want to connect.
> **/
> -int setup_serve(SERVER *serve) {
> +int setup_serve(SERVER *const serve, GError **const gerror) {
> struct addrinfo hints;
> struct addrinfo *ai = NULL;
> gchar *port = NULL;
> int e;
> - GError *gerror = NULL;
>
> /* Without this, it's possible that socket == 0, even if it's
> * not initialized at all. And that would be wrong because 0 is
> @@ -2432,10 +2431,12 @@ int setup_serve(SERVER *serve) {
> g_free(port);
>
> if(e != 0) {
> - fprintf(stderr, "getaddrinfo failed: %s\n", gai_strerror(e));
> - serve->socket = -1;
> - freeaddrinfo(ai);
> - exit(EXIT_FAILURE);
> + g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_GAI,
> + "failed to open an export socket: "
> + "failed to get address info: %s",
> + gai_strerror(e));
> + freeaddrinfo(ai);
> + return -1;
> }
Same here, obviously.
> if(serve->socket_family == AF_UNSPEC)
> @@ -2449,23 +2450,46 @@ int setup_serve(SERVER *serve) {
> ai->ai_family = AF_INET6_SDP;
> }
> #endif
> - if ((serve->socket = socket(ai->ai_family, ai->ai_socktype,
> ai->ai_protocol)) < 0)
> - err("socket: %m");
> -
> - if (dosockopts(serve->socket, &gerror) == -1) {
> - msg(LOG_ERR, "failed to open export-specific socket: %s",
> - gerror->message);
> - g_clear_error(&gerror);
> - exit(EXIT_FAILURE);
> + if ((serve->socket = socket(ai->ai_family, ai->ai_socktype,
> ai->ai_protocol)) < 0) {
> + g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_SOCKET,
> + "failed to open an export socket: "
> + "failed to create a socket: %s",
> + strerror(errno));
> + freeaddrinfo(ai);
> + return -1;
> + }
> +
> + if (dosockopts(serve->socket, gerror) == -1) {
> + g_prefix_error(gerror, "failed to open an export socket: ");
> + close(serve->socket);
> + serve->socket = -1;
> + freeaddrinfo(ai);
> + return -1;
> }
>
> DEBUG("Waiting for connections... bind, ");
> e = bind(serve->socket, ai->ai_addr, ai->ai_addrlen);
> - if (e != 0 && errno != EADDRINUSE)
> - err("bind: %m");
> + if (e != 0 && errno != EADDRINUSE) {
> + g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_BIND,
> + "failed to open an export socket: "
> + "failed to bind an address to a socket: %s",
> + strerror(errno));
> + close(serve->socket);
> + serve->socket = -1;
> + freeaddrinfo(ai);
> + return -1;
> + }
> DEBUG("listen, ");
> - if (listen(serve->socket, 1) < 0)
> - err("listen: %m");
> + if (listen(serve->socket, 1) < 0) {
> + g_set_error(gerror, SETUP_ERROR, SETUP_ERROR_BIND,
> + "failed to open an export socket: "
> + "failed to start listening on a socket: %s",
> + strerror(errno));
> + close(serve->socket);
> + serve->socket = -1;
> + freeaddrinfo(ai);
> + return -1;
> + }
>
> freeaddrinfo (ai);
> if(serve->servename) {
> @@ -2551,7 +2575,18 @@ void setup_servers(GArray *const servers, const gchar
> *const modernaddr,
> int want_modern=0;
>
> for(i=0;i<servers->len;i++) {
> - want_modern |= setup_serve(&(g_array_index(servers, SERVER,
> i)));
> + GError *gerror = NULL;
> + SERVER *server = &g_array_index(servers, SERVER, i);
> + int ret;
> +
> + ret = setup_serve(server, &gerror);
> + if (ret == -1) {
> + msg(LOG_ERR, "failed to setup servers: %s",
> + gerror->message);
> + g_clear_error(&gerror);
> + exit(EXIT_FAILURE);
> + }
> + want_modern |= ret;
> }
> if(want_modern) {
> GError *gerror = NULL;
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
> much more. Get web development skills now with LearnDevNow -
> 350+ hours of 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_122812
> _______________________________________________
> Nbd-general mailing list
> [email protected]
> 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.
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/nbd-general
