missing freeing previous data struction when new allocation fails

林昊翔 linhaoxiang at gmail.com
Wed Nov 17 19:16:24 PST 2010


The scenario is:
struct A *foo()
{
    A *a;

    a = malloc(sizeof(A));
    if (a == NULL)
        return NULL;

    a->b = malloc(sizeof(B));
    if (a->b == NULL)
        return NULL;

    return a;
}

There should be "free(a);" before the red line.

I find that there are many such ommissions in the wayland source tree. The
attached txt shows part of them.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20101118/5c848220/attachment.htm>
-------------- next part --------------
diff -Nur wayland/compositor/compositor-drm.c wayland-copy/compositor/compositor-drm.c
--- wayland/compositor/compositor-drm.c	2010-11-18 10:10:46.680032600 +0800
+++ wayland-copy/compositor/compositor-drm.c	2010-11-18 10:54:42.738612100 +0800
@@ -243,6 +243,10 @@
 	wlsc_input_device_init(&input->base, &c->base);
 
 	e = udev_enumerate_new(c->udev);
+	if (e == NULL) {
+		free(input);
+		return;
+	}
 	udev_enumerate_add_match_subsystem(e, "input");
 	udev_enumerate_add_match_property(e, "WAYLAND_SEAT", "1");
         udev_enumerate_scan_devices(e);
@@ -395,6 +399,7 @@
 	encoder = drmModeGetEncoder(ec->base.drm.fd, connector->encoders[0]);
 	if (encoder == NULL) {
 		fprintf(stderr, "No encoder for connector.\n");
+		free(output);
 		return -1;
 	}
 
@@ -404,6 +409,7 @@
 	}
 	if (i == resources->count_crtcs) {
 		fprintf(stderr, "No usable crtc for encoder.\n");
+		free(output);
 		return -1;
 	}
 
@@ -435,6 +441,7 @@
 				   32, 32, stride, handle, &output->fb_id[i]);
 		if (ret) {
 			fprintf(stderr, "failed to add fb %d: %m\n", i);
+			free(output);
 			return -1;
 		}
 	}
@@ -449,6 +456,7 @@
 			     &output->connector_id, 1, &output->mode);
 	if (ret) {
 		fprintf(stderr, "failed to set mode: %m\n");
+		free(output);
 		return -1;
 	}
 
@@ -645,10 +653,13 @@
 	ec->udev = udev_new();
 	if (ec->udev == NULL) {
 		fprintf(stderr, "failed to initialize udev context\n");
+		free(ec);
 		return NULL;
 	}
 
 	e = udev_enumerate_new(ec->udev);
+	if (e == NULL) {
+	}
 	udev_enumerate_add_match_subsystem(e, "drm");
 	udev_enumerate_add_match_property(e, "WAYLAND_SEAT", "1");
         udev_enumerate_scan_devices(e);
@@ -662,21 +673,29 @@
 
 	if (device == NULL) {
 		fprintf(stderr, "no drm device found\n");
+		free(ec->udev);
+		free(ec);
 		return NULL;
 	}
 
 	ec->base.wl_display = display;
 	if (init_egl(ec, device) < 0) {
 		fprintf(stderr, "failed to initialize egl\n");
+		free(ec->udev);
+		free(ec);
 		return NULL;
 	}
 	
 	/* Can't init base class until we have a current egl context */
 	if (wlsc_compositor_init(&ec->base, display) < 0)
+		free(ec->udev);
+		free(ec);
 		return NULL;
 
 	if (create_outputs(ec, connector) < 0) {
 		fprintf(stderr, "failed to create output for %s\n", path);
+		free(ec->udev);
+		free(ec);
 		return NULL;
 	}
 
@@ -686,6 +705,10 @@
 	ec->drm_source =
 		wl_event_loop_add_fd(loop, ec->base.drm.fd,
 				     WL_EVENT_READABLE, on_drm_input, ec);
+	if (ec->drm_source == NULL) {
+		free(ec->udev);
+		free(ec);
+	}
 	setup_tty(ec, loop);
 	ec->base.authenticate = drm_authenticate;
 	ec->base.present = drm_compositor_present;
diff -Nur wayland/compositor/compositor-x11.c wayland-copy/compositor/compositor-x11.c
--- wayland/compositor/compositor-x11.c	2010-11-18 10:10:46.681032700 +0800
+++ wayland-copy/compositor/compositor-x11.c	2010-11-18 10:57:17.284065100 +0800
@@ -413,15 +413,22 @@
 						 output->window,
 						 1, 1, attachments);
 	reply = xcb_dri2_get_buffers_reply (c->conn, cookie, NULL);
-	if (reply == NULL)
+	if (reply == NULL) {
+		free(output);
 		return -1;
+	}
 	buffers = xcb_dri2_get_buffers_buffers (reply);
-	if (buffers == NULL)
+	if (buffers == NULL) {
+		free(reply);
+		free(output);
 		return -1;
+	}
 
 	if (reply->count != 1) {
 		fprintf(stderr,
 			"got wrong number of buffers (%d)\n", reply->count);
+		free(reply);
+		free(output);
 		return -1;
 	}
 
diff -Nur wayland/wayland/connection.c wayland-copy/wayland/connection.c
--- wayland/wayland/connection.c	2010-11-18 10:10:46.722036800 +0800
+++ wayland-copy/wayland/connection.c	2010-11-18 10:23:30.671424100 +0800
@@ -160,6 +160,9 @@
 	struct wl_connection *connection;
 
 	connection = malloc(sizeof *connection);
+	if (connection == NULL) {
+		return NULL;
+	}
 	memset(connection, 0, sizeof *connection);
 	connection->fd = fd;
 	connection->update = update;
diff -Nur wayland/wayland/wayland-client.c wayland-copy/wayland/wayland-client.c
--- wayland/wayland/wayland-client.c	2010-11-18 10:10:46.733037900 +0800
+++ wayland-copy/wayland/wayland-client.c	2010-11-18 10:25:33.851740900 +0800
@@ -366,6 +366,11 @@
 	}
 
 	display->objects = wl_hash_table_create();
+	if (display->objects == NULL) {
+		close(display->fd);
+		free(display);
+		return NULL;
+	}
 	wl_list_init(&display->global_listener_list);
 
 	display->proxy.base.interface = &wl_display_interface;
@@ -382,6 +387,12 @@
 	display->connection = wl_connection_create(display->fd,
 						   connection_update,
 						   display);
+	if (display->connection == NULL) {
+		wl_hash_table_destroy(display->objects);
+		close(display->fd);
+		free(display);
+		return NULL;
+	}
 
 	return display;
 }
@@ -390,6 +401,7 @@
 wl_display_destroy(struct wl_display *display)
 {
 	wl_connection_destroy(display->connection);
+	wl_hash_table_destroy(display->objects);
 	close(display->fd);
 	free(display);
 }
diff -Nur wayland/wayland/wayland-server.c wayland-copy/wayland/wayland-server.c
--- wayland/wayland/wayland-server.c	2010-11-18 10:10:46.739038500 +0800
+++ wayland-copy/wayland/wayland-server.c	2010-11-18 10:20:00.165375600 +0800
@@ -368,6 +368,7 @@
 
 	display->objects = wl_hash_table_create();
 	if (display->objects == NULL) {
+		wl_event_loop_destroy(display->loop);
 		free(display);
 		return NULL;
 	}
@@ -382,6 +383,7 @@
 	display->base.implementation = (void (**)(void)) &display_interface;
 	wl_display_add_object(display, &display->base);
 	if (wl_display_add_global(display, &display->base, NULL)) {
+		wl_hash_table_destroy(display->objects);
 		wl_event_loop_destroy(display->loop);
 		free(display);
 		return NULL;


More information about the wayland-devel mailing list