[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[David Goulet]

Signed-off-by: David Goulet <david.goulet at polymtl.ca>
---
 libust/tracectl.c    |   63 ++++++++++++++++++++++++++++++++++++++++++--------
 libustcmd/ustcmd.c   |   41 +++++++++++++++++++++++++++-----
 libustcomm/ustcomm.c |   24 ++++++++++++++++---
 libustd/libustd.c    |   37 ++++++++++++++++++++++++----
 tests/hello/hello.c  |    6 +++-
 ustd/ustd.c          |   12 ++++++++-
 6 files changed, 152 insertions(+), 31 deletions(-)

diff --git a/libust/tracectl.c b/libust/tracectl.c
index ac551d5..1bb841f 100644
--- a/libust/tracectl.c
+++ b/libust/tracectl.c
@@ -156,7 +156,11 @@ static void inform_consumer_daemon(const char *trace_name)
 			/* iterate on all cpus */
 			for(j=0; j<trace->channels[i].n_cpus; j++) {
 				char *buf;
-				asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j);
+				if(asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j) == -1) {
+					ERR("inform_consumer_daemon : asprintf failed (%s_%d)",
+							trace->channels[i].channel_name, j);
+					goto finish;
+				}
 				result = ustcomm_request_consumer(pid, buf);
 				if(result == -1) {
 					WARN("Failed to request collection for channel %s. Is the daemon available?", trace->channels[i].channel_name);
@@ -213,7 +217,11 @@ int process_blkd_consumer_act(void *priv, int fd, short events)
 	else if(result < 0) {
 		ERR("ust_buffers_get_subbuf: error: %s", strerror(-result));
 	}
-	asprintf(&reply, "%s %ld", "OK", consumed_old);
+	if(asprintf(&reply, "%s %ld", "OK", consumed_old) == -1) {
+		ERR("process_blkd_consumer_act : asprintf failed (OK %ld)",
+				consumed_old);
+		return -1;
+	}
 	result = ustcomm_send_reply(&bc->server, reply, &bc->src);
 	if(result < 0) {
 		ERR("ustcomm_send_reply failed");
@@ -250,7 +258,11 @@ void seperate_channel_cpu(const char *channel_and_cpu, char **channel, int *cpu)
 		*cpu = atoi(sep+1);
 	}
 
-	asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu);
+	if(asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu) == -1) {
+		ERR("seperate_channel_cpu : asprintf failed (%.*s)",
+				(int)(sep-channel_and_cpu), channel_and_cpu);
+		return;
+	}
 }
 
 static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
@@ -298,7 +310,12 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
 
 //			DBG("the shmid for the requested channel is %d", buf->shmid);
 //			DBG("the shmid for its buffer structure is %d", channel->buf_struct_shmids);
-			asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]);
+			if(asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]) == -1) {
+				ERR("do_cmd_get_shmid : asprintf failed (%d %d)",
+						buf->shmid, channel->buf_struct_shmids[ch_cpu]);
+				retval = -1;
+				goto free_short_chan_name;
+			}
 
 			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
 			if(result) {
@@ -369,7 +386,12 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
 			char *reply;
 
 			DBG("the n_subbufs for the requested channel is %d", channel->subbuf_cnt);
-			asprintf(&reply, "%d", channel->subbuf_cnt);
+			if(asprintf(&reply, "%d", channel->subbuf_cnt) == -1) {
+				ERR("do_cmd_get_n_subbufs : asprintf failed (%d)",
+						channel->subbuf_cnt);
+				retval = -1;
+				goto free_short_chan_name;
+			}
 
 			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
 			if(result) {
@@ -438,7 +460,12 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
 			char *reply;
 
 			DBG("the subbuf_size for the requested channel is %zd", channel->subbuf_size);
-			asprintf(&reply, "%zd", channel->subbuf_size);
+			if(asprintf(&reply, "%zd", channel->subbuf_size) == -1) {
+				ERR("do_cmd_get_subbuf_size : asprintf failed (%zd)",
+						channel->subbuf_size);
+				retval = -1;
+				goto free_short_chan_name;
+			}
 
 			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
 			if(result) {
@@ -746,11 +773,19 @@ static int do_cmd_put_subbuffer(const char *recvbuf, struct ustcomm_source *src)
 			result = ust_buffers_put_subbuf(buf, consumed_old);
 			if(result < 0) {
 				WARN("ust_buffers_put_subbuf: error (subbuf=%s)", channel_and_cpu);
-				asprintf(&reply, "%s", "ERROR");
+				if(asprintf(&reply, "%s", "ERROR") == -1) {
+					ERR("do_cmd_put_subbuffer : asprintf failed (ERROR)");
+					retval = -1;
+					goto unlock_traces;
+				}
 			}
 			else {
 				DBG("ust_buffers_put_subbuf: success (subbuf=%s)", channel_and_cpu);
-				asprintf(&reply, "%s", "OK");
+				if(asprintf(&reply, "%s", "OK") == -1) {
+					ERR("do_cmd_put_subbuffer : asprintf failed (OK)");
+					retval = -1;
+					goto unlock_traces;
+				}
 			}
 
 			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
@@ -992,7 +1027,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
 	else if(nth_token_is(recvbuf, "get_pidunique", 0) == 1) {
 		char *reply;
 
-		asprintf(&reply, "%lld", pidunique);
+		if(asprintf(&reply, "%lld", pidunique) == -1) {
+			ERR("process_client_cmd : asprintf failed (%lld)",
+					pidunique);
+			goto next_cmd;
+		}
 
 		result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
 		if(result) {
@@ -1005,7 +1044,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
 	else if(nth_token_is(recvbuf, "get_sock_path", 0) == 1) {
 		char *reply = getenv("UST_DAEMON_SOCKET");
 		if(!reply) {
-			asprintf(&reply, "%s/%s", SOCK_DIR, "ustd");
+			if(asprintf(&reply, "%s/%s", SOCK_DIR, "ustd") == -1) {
+				ERR("process_client_cmd : asprintf failed (%s/ustd)",
+						SOCK_DIR);
+				goto next_cmd;
+			}
 			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
 			free(reply);
 		}
diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
index cf6b9d7..1d35288 100644
--- a/libustcmd/ustcmd.c
+++ b/libustcmd/ustcmd.c
@@ -90,7 +90,11 @@ int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
 		return USTCMD_ERR_ARG;
 	}
 
-	asprintf(&cmd, "%s %s", cmd_str[state], mn);
+	if(asprintf(&cmd, "%s %s", cmd_str[state], mn) == -1) {
+		ERR("ustcmd_set_marker_state : asprintf failed (%s %s)",
+				cmd_str[state], mn);
+		return USTCMD_ERR_GEN;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, NULL);
 	if (result) {
@@ -114,7 +118,11 @@ int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
 	char *cmd;
 	int result;
 
-	asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size);
+	if(asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size) == -1) {
+		ERR("ustcmd_set_subbuf_size : asprintf failed (set_subbuf_size %s)",
+				channel_size);
+		return -1;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, NULL);
 	if (result != 1) {
@@ -138,7 +146,11 @@ int ustcmd_set_subbuf_num(const char *channel_size, pid_t pid)
 	char *cmd;
 	int result;
 
-	asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size);
+	if(asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size) == -1) {
+		ERR("ustcmd_set_subbuf_num : asprintf failed (set_subbuf_num %s",
+				channel_size);
+		return -1;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, NULL);
 	if (result != 1) {
@@ -163,7 +175,11 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
 	int result;
 
 	/* format: channel_cpu */
-	asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel);
+	if(asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel) == -1) {
+		ERR("ustcmd_get_subbuf_size : asprintf failed (get_subbuf_size, %s_0",
+				channel);
+		return -1;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, &reply);
 	if (result) {
@@ -191,7 +207,11 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
 	int result;
 
 	/* format: channel_cpu */
-	asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel);
+	if(asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel) == -1) {
+		ERR("ustcmd_get_subbuf_num : asprintf failed (get_n_subbufs, %s_0",
+				channel);
+		return -1;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, &reply);
 	if (result) {
@@ -432,7 +452,11 @@ int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
 	char *cmd;
 	int result;
 
-	asprintf(&cmd, "%s %s", "set_sock_path", sock_path);
+	if(asprintf(&cmd, "%s %s", "set_sock_path", sock_path) == -1) {
+		ERR("ustcmd_set_sock_path : asprintf failed (set_sock_path, %s",
+				sock_path);
+		return -1;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, NULL);
 	if (result != 1) {
@@ -456,7 +480,10 @@ int ustcmd_get_sock_path(char **sock_path, pid_t pid)
 	char *cmd, *reply;
 	int result;
 
-	asprintf(&cmd, "%s", "get_sock_path");
+	if(asprintf(&cmd, "%s", "get_sock_path") == -1) {
+		ERR("ustcmd_get_sock_path : asprintf failed");
+		return USTCMD_ERR_GEN;
+	}
 
 	result = ustcmd_send_cmd(cmd, pid, &reply);
 	if (result != 1) {
diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
index f2b7a03..30c8f24 100644
--- a/libustcomm/ustcomm.c
+++ b/libustcomm/ustcomm.c
@@ -163,7 +163,11 @@ int ustcomm_request_consumer(pid_t pid, const char *channel)
 		return -1;
 	}
 
-	asprintf(&msg, "collect %d %s", pid, channel); 
+	if(asprintf(&msg, "collect %d %s", pid, channel) == -1) {
+		ERR("ustcomm_request_consumer : asprintf failed (collect %d/%s)",
+				pid, channel);
+		return -1;
+	}
 
 	/* don't signal it because it's the daemon */
 	result = ustcomm_connect_path(path, &conn, -1);
@@ -706,7 +710,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
 	int retval = 0;
 
 	if(sock_path) {
-		asprintf(&name, "%s", sock_path);
+		if(asprintf(&name, "%s", sock_path) == -1) {
+			ERR("ustcomm_init_ustd : asprintf failed (sock_path %s)",
+					sock_path);
+			return -1;
+		}
 	}
 	else {
 		int result;
@@ -718,7 +726,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
 			return -1;
 		}
 
-		asprintf(&name, "%s/%s", SOCK_DIR, "ustd");
+		if(asprintf(&name, "%s/%s", SOCK_DIR, "ustd") == -1) {
+			ERR("ustcomm_init_ustd : asprintf failed (%s/ustd)",
+					SOCK_DIR);
+			return -1;
+		}
 	}
 
 	handle->server.listen_fd = init_named_socket(name, &handle->server.socketpath);
@@ -864,7 +876,11 @@ char *nth_token(const char *str, int tok_no)
 		retval = NULL;
 	}
 
-	asprintf(&retval, "%.*s", (int)(end-start), start);
+	if(asprintf(&retval, "%.*s", (int)(end-start), start) == -1) {
+		ERR("nth_token : asprintf failed (%.*s)",
+				(int)(end-start), start);
+		return NULL;
+	}
 
 	return retval;
 }
diff --git a/libustd/libustd.c b/libustd/libustd.c
index 4935131..cc88cde 100644
--- a/libustd/libustd.c
+++ b/libustd/libustd.c
@@ -58,7 +58,12 @@ int get_subbuffer(struct buffer_info *buf)
 	int retval;
 	int result;
 
-	asprintf(&send_msg, "get_subbuffer %s", buf->name);
+	if(asprintf(&send_msg, "get_subbuffer %s", buf->name) == -1) {
+		ERR("get_subbuffer : asprintf failed (%s)",
+				buf->name);
+		retval = -1;
+		goto end;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	if((result == -1 && (errno == ECONNRESET || errno == EPIPE)) || result == 0) {
 		DBG("app died while being traced");
@@ -118,7 +123,12 @@ int put_subbuffer(struct buffer_info *buf)
 	int retval;
 	int result;
 
-	asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old);
+	if(asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old) == -1) {
+		ERR("put_subbuffer : asprintf failed (%s %ld)",
+				buf->name, buf->consumed_old);
+		retval = -1;
+		goto end;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	if(result < 0 && (errno == ECONNRESET || errno == EPIPE)) {
 		retval = PUT_SUBBUF_DIED;
@@ -215,7 +225,10 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
 	}
 
 	/* get pidunique */
-	asprintf(&send_msg, "get_pidunique");
+	if(asprintf(&send_msg, "get_pidunique") == -1) {
+		ERR("connect_buffer : asprintf failed (get_pidunique)");
+		return NULL;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	free(send_msg);
 	if(result == -1) {
@@ -235,7 +248,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
 	DBG("got pidunique %lld", buf->pidunique);
 
 	/* get shmid */
-	asprintf(&send_msg, "get_shmid %s", buf->name);
+	if(asprintf(&send_msg, "get_shmid %s", buf->name) == -1) {
+		ERR("connect_buffer : asprintf failed (get_schmid %s)",
+				buf->name);
+		return NULL;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	free(send_msg);
 	if(result == -1) {
@@ -255,7 +272,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
 	DBG("got shmids %d %d", buf->shmid, buf->bufstruct_shmid);
 
 	/* get n_subbufs */
-	asprintf(&send_msg, "get_n_subbufs %s", buf->name);
+	if(asprintf(&send_msg, "get_n_subbufs %s", buf->name) == -1) {
+		ERR("connect_buffer : asprintf failed (get_n_subbufs %s)",
+				buf->name);
+		return NULL;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	free(send_msg);
 	if(result == -1) {
@@ -275,7 +296,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
 	DBG("got n_subbufs %d", buf->n_subbufs);
 
 	/* get subbuf size */
-	asprintf(&send_msg, "get_subbuf_size %s", buf->name);
+	if(asprintf(&send_msg, "get_subbuf_size %s", buf->name) == -1) {
+		ERR("connect_buffer : asprintf failed (get_subbuf_size %s)",
+				buf->name);
+		return NULL;
+	}
 	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
 	free(send_msg);
 	if(result == -1) {
diff --git a/tests/hello/hello.c b/tests/hello/hello.c
index 8147860..f58706b 100644
--- a/tests/hello/hello.c
+++ b/tests/hello/hello.c
@@ -77,13 +77,15 @@ int main()
 		usleep(100000);
 	}
 
-	scanf("%*s");
+	if(scanf("%*s") == EOF)
+		PERROR("scanf failed");
 
 	ltt_trace_stop("auto");
 	ltt_trace_destroy("auto", 0);
 
 	DBG("TRACE STOPPED");
-	scanf("%*s");
+	if(scanf("%*s") == EOF)
+		PERROR("scanf failed");
 
 	return 0;
 }
diff --git a/ustd/ustd.c b/ustd/ustd.c
index c0f4c59..f15c31a 100644
--- a/ustd/ustd.c
+++ b/ustd/ustd.c
@@ -191,7 +191,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
 		trace_path = USTD_DEFAULT_TRACE_PATH;
 	}
 
-	asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique);
+	if(asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique) == -1) {
+		ERR("on_open_buffer : asprintf failed (%s/%u_%lld)",
+				trace_path, buf->pid, buf->pidunique);
+		return 1;
+	}
 	result = create_dir_if_needed(tmp);
 	if(result == -1) {
 		ERR("could not create directory %s", tmp);
@@ -200,7 +204,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
 	}
 	free(tmp);
 
-	asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name);
+	if(asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name) == -1) {
+		ERR("on_open_buffer : asprintf failed (%s/%u_%lld/%s)",
+				trace_path, buf->pid, buf->pidunique, buf->name);
+		return 1;
+	}
 	result = fd = open(tmp, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 00600);
 	if(result == -1) {
 		PERROR("open");
-- 
1.7.0.4

[ltt-dev] [PATCH 2/2] Fix retval value assignment on error

[David Goulet]

Signed-off-by: David Goulet <david.goulet at polymtl.ca>
---
 libust/tracectl.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/libust/tracectl.c b/libust/tracectl.c
index 1bb841f..909cbe8 100644
--- a/libust/tracectl.c
+++ b/libust/tracectl.c
@@ -282,12 +282,14 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
 	channel_and_cpu = nth_token(recvbuf, 1);
 	if(channel_and_cpu == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 
 	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
 	if(ch_cpu == -1) {
 		ERR("problem parsing channel name");
+		retval = -1;
 		goto free_short_chan_name;
 	}
 
@@ -360,12 +362,14 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
 	channel_and_cpu = nth_token(recvbuf, 1);
 	if(channel_and_cpu == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 
 	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
 	if(ch_cpu == -1) {
 		ERR("problem parsing channel name");
+		retval = -1;
 		goto free_short_chan_name;
 	}
 
@@ -434,12 +438,14 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
 	channel_and_cpu = nth_token(recvbuf, 1);
 	if(channel_and_cpu == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 
 	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
 	if(ch_cpu == -1) {
 		ERR("problem parsing channel name");
+		retval = -1;
 		goto free_short_chan_name;
 	}
 
@@ -522,6 +528,7 @@ static int do_cmd_set_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
 
 	if(ch_name == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 
@@ -577,10 +584,12 @@ static int do_cmd_set_subbuf_num(const char *recvbuf, struct ustcomm_source *src
 
 	if(ch_name == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 	if (num < 2) {
 		ERR("subbuffer count should be greater than 2");
+		retval = -1;
 		goto end;
 	}
 
@@ -629,12 +638,14 @@ static int do_cmd_get_subbuffer(const char *recvbuf, struct ustcomm_source *src)
 	channel_and_cpu = nth_token(recvbuf, 1);
 	if(channel_and_cpu == NULL) {
 		ERR("cannot parse channel");
+		retval = -1;
 		goto end;
 	}
 
 	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
 	if(ch_cpu == -1) {
 		ERR("problem parsing channel name");
+		retval = -1;
 		goto free_short_chan_name;
 	}
 
-- 
1.7.0.4

[ltt-dev] [PATCH 2/2] Fix retval value assignment on error

[Mathieu Desnoyers]

* David Goulet (david.goulet at polymtl.ca) wrote:
> Signed-off-by: David Goulet <david.goulet at polymtl.ca>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

> ---
>  libust/tracectl.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index 1bb841f..909cbe8 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -282,12 +282,14 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
>  	channel_and_cpu = nth_token(recvbuf, 1);
>  	if(channel_and_cpu == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  
>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>  	if(ch_cpu == -1) {
>  		ERR("problem parsing channel name");
> +		retval = -1;
>  		goto free_short_chan_name;
>  	}
>  
> @@ -360,12 +362,14 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
>  	channel_and_cpu = nth_token(recvbuf, 1);
>  	if(channel_and_cpu == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  
>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>  	if(ch_cpu == -1) {
>  		ERR("problem parsing channel name");
> +		retval = -1;
>  		goto free_short_chan_name;
>  	}
>  
> @@ -434,12 +438,14 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>  	channel_and_cpu = nth_token(recvbuf, 1);
>  	if(channel_and_cpu == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  
>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>  	if(ch_cpu == -1) {
>  		ERR("problem parsing channel name");
> +		retval = -1;
>  		goto free_short_chan_name;
>  	}
>  
> @@ -522,6 +528,7 @@ static int do_cmd_set_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>  
>  	if(ch_name == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  
> @@ -577,10 +584,12 @@ static int do_cmd_set_subbuf_num(const char *recvbuf, struct ustcomm_source *src
>  
>  	if(ch_name == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  	if (num < 2) {
>  		ERR("subbuffer count should be greater than 2");
> +		retval = -1;
>  		goto end;
>  	}
>  
> @@ -629,12 +638,14 @@ static int do_cmd_get_subbuffer(const char *recvbuf, struct ustcomm_source *src)
>  	channel_and_cpu = nth_token(recvbuf, 1);
>  	if(channel_and_cpu == NULL) {
>  		ERR("cannot parse channel");
> +		retval = -1;
>  		goto end;
>  	}
>  
>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>  	if(ch_cpu == -1) {
>  		ERR("problem parsing channel name");
> +		retval = -1;
>  		goto free_short_chan_name;
>  	}
>  
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[Mathieu Desnoyers]

* David Goulet (david.goulet at polymtl.ca) wrote:
> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
> ---
>  libust/tracectl.c    |   63 ++++++++++++++++++++++++++++++++++++++++++--------
>  libustcmd/ustcmd.c   |   41 +++++++++++++++++++++++++++-----
>  libustcomm/ustcomm.c |   24 ++++++++++++++++---
>  libustd/libustd.c    |   37 ++++++++++++++++++++++++----
>  tests/hello/hello.c  |    6 +++-
>  ustd/ustd.c          |   12 ++++++++-
>  6 files changed, 152 insertions(+), 31 deletions(-)
> 
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index ac551d5..1bb841f 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -156,7 +156,11 @@ static void inform_consumer_daemon(const char *trace_name)
>  			/* iterate on all cpus */
>  			for(j=0; j<trace->channels[i].n_cpus; j++) {
>  				char *buf;
> -				asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j);
> +				if(asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j) == -1) {

please standardize on the kernel coding style:

if ()
for ()
switch ()

but function() and macroname().

Also, please use a if (asprintf(...) < 0) test. I know the manpage
states that -1 is explicitely returned, but it's very typical for all
negative values to be more explicit error value variants. So let's make
these tests as similar as possible.

Thanks,

Mathieu

> +					ERR("inform_consumer_daemon : asprintf failed (%s_%d)",
> +							trace->channels[i].channel_name, j);
> +					goto finish;
> +				}
>  				result = ustcomm_request_consumer(pid, buf);
>  				if(result == -1) {
>  					WARN("Failed to request collection for channel %s. Is the daemon available?", trace->channels[i].channel_name);
> @@ -213,7 +217,11 @@ int process_blkd_consumer_act(void *priv, int fd, short events)
>  	else if(result < 0) {
>  		ERR("ust_buffers_get_subbuf: error: %s", strerror(-result));
>  	}
> -	asprintf(&reply, "%s %ld", "OK", consumed_old);
> +	if(asprintf(&reply, "%s %ld", "OK", consumed_old) == -1) {
> +		ERR("process_blkd_consumer_act : asprintf failed (OK %ld)",
> +				consumed_old);
> +		return -1;
> +	}
>  	result = ustcomm_send_reply(&bc->server, reply, &bc->src);
>  	if(result < 0) {
>  		ERR("ustcomm_send_reply failed");
> @@ -250,7 +258,11 @@ void seperate_channel_cpu(const char *channel_and_cpu, char **channel, int *cpu)
>  		*cpu = atoi(sep+1);
>  	}
>  
> -	asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu);
> +	if(asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu) == -1) {
> +		ERR("seperate_channel_cpu : asprintf failed (%.*s)",
> +				(int)(sep-channel_and_cpu), channel_and_cpu);
> +		return;
> +	}
>  }
>  
>  static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
> @@ -298,7 +310,12 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
>  
>  //			DBG("the shmid for the requested channel is %d", buf->shmid);
>  //			DBG("the shmid for its buffer structure is %d", channel->buf_struct_shmids);
> -			asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]);
> +			if(asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]) == -1) {
> +				ERR("do_cmd_get_shmid : asprintf failed (%d %d)",
> +						buf->shmid, channel->buf_struct_shmids[ch_cpu]);
> +				retval = -1;
> +				goto free_short_chan_name;
> +			}
>  
>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>  			if(result) {
> @@ -369,7 +386,12 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
>  			char *reply;
>  
>  			DBG("the n_subbufs for the requested channel is %d", channel->subbuf_cnt);
> -			asprintf(&reply, "%d", channel->subbuf_cnt);
> +			if(asprintf(&reply, "%d", channel->subbuf_cnt) == -1) {
> +				ERR("do_cmd_get_n_subbufs : asprintf failed (%d)",
> +						channel->subbuf_cnt);
> +				retval = -1;
> +				goto free_short_chan_name;
> +			}
>  
>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>  			if(result) {
> @@ -438,7 +460,12 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>  			char *reply;
>  
>  			DBG("the subbuf_size for the requested channel is %zd", channel->subbuf_size);
> -			asprintf(&reply, "%zd", channel->subbuf_size);
> +			if(asprintf(&reply, "%zd", channel->subbuf_size) == -1) {
> +				ERR("do_cmd_get_subbuf_size : asprintf failed (%zd)",
> +						channel->subbuf_size);
> +				retval = -1;
> +				goto free_short_chan_name;
> +			}
>  
>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>  			if(result) {
> @@ -746,11 +773,19 @@ static int do_cmd_put_subbuffer(const char *recvbuf, struct ustcomm_source *src)
>  			result = ust_buffers_put_subbuf(buf, consumed_old);
>  			if(result < 0) {
>  				WARN("ust_buffers_put_subbuf: error (subbuf=%s)", channel_and_cpu);
> -				asprintf(&reply, "%s", "ERROR");
> +				if(asprintf(&reply, "%s", "ERROR") == -1) {
> +					ERR("do_cmd_put_subbuffer : asprintf failed (ERROR)");
> +					retval = -1;
> +					goto unlock_traces;
> +				}
>  			}
>  			else {
>  				DBG("ust_buffers_put_subbuf: success (subbuf=%s)", channel_and_cpu);
> -				asprintf(&reply, "%s", "OK");
> +				if(asprintf(&reply, "%s", "OK") == -1) {
> +					ERR("do_cmd_put_subbuffer : asprintf failed (OK)");
> +					retval = -1;
> +					goto unlock_traces;
> +				}
>  			}
>  
>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
> @@ -992,7 +1027,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
>  	else if(nth_token_is(recvbuf, "get_pidunique", 0) == 1) {
>  		char *reply;
>  
> -		asprintf(&reply, "%lld", pidunique);
> +		if(asprintf(&reply, "%lld", pidunique) == -1) {
> +			ERR("process_client_cmd : asprintf failed (%lld)",
> +					pidunique);
> +			goto next_cmd;
> +		}
>  
>  		result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>  		if(result) {
> @@ -1005,7 +1044,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
>  	else if(nth_token_is(recvbuf, "get_sock_path", 0) == 1) {
>  		char *reply = getenv("UST_DAEMON_SOCKET");
>  		if(!reply) {
> -			asprintf(&reply, "%s/%s", SOCK_DIR, "ustd");
> +			if(asprintf(&reply, "%s/%s", SOCK_DIR, "ustd") == -1) {
> +				ERR("process_client_cmd : asprintf failed (%s/ustd)",
> +						SOCK_DIR);
> +				goto next_cmd;
> +			}
>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>  			free(reply);
>  		}
> diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
> index cf6b9d7..1d35288 100644
> --- a/libustcmd/ustcmd.c
> +++ b/libustcmd/ustcmd.c
> @@ -90,7 +90,11 @@ int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
>  		return USTCMD_ERR_ARG;
>  	}
>  
> -	asprintf(&cmd, "%s %s", cmd_str[state], mn);
> +	if(asprintf(&cmd, "%s %s", cmd_str[state], mn) == -1) {
> +		ERR("ustcmd_set_marker_state : asprintf failed (%s %s)",
> +				cmd_str[state], mn);
> +		return USTCMD_ERR_GEN;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>  	if (result) {
> @@ -114,7 +118,11 @@ int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
>  	char *cmd;
>  	int result;
>  
> -	asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size);
> +	if(asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size) == -1) {
> +		ERR("ustcmd_set_subbuf_size : asprintf failed (set_subbuf_size %s)",
> +				channel_size);
> +		return -1;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>  	if (result != 1) {
> @@ -138,7 +146,11 @@ int ustcmd_set_subbuf_num(const char *channel_size, pid_t pid)
>  	char *cmd;
>  	int result;
>  
> -	asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size);
> +	if(asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size) == -1) {
> +		ERR("ustcmd_set_subbuf_num : asprintf failed (set_subbuf_num %s",
> +				channel_size);
> +		return -1;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>  	if (result != 1) {
> @@ -163,7 +175,11 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
>  	int result;
>  
>  	/* format: channel_cpu */
> -	asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel);
> +	if(asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel) == -1) {
> +		ERR("ustcmd_get_subbuf_size : asprintf failed (get_subbuf_size, %s_0",
> +				channel);
> +		return -1;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>  	if (result) {
> @@ -191,7 +207,11 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
>  	int result;
>  
>  	/* format: channel_cpu */
> -	asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel);
> +	if(asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel) == -1) {
> +		ERR("ustcmd_get_subbuf_num : asprintf failed (get_n_subbufs, %s_0",
> +				channel);
> +		return -1;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>  	if (result) {
> @@ -432,7 +452,11 @@ int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
>  	char *cmd;
>  	int result;
>  
> -	asprintf(&cmd, "%s %s", "set_sock_path", sock_path);
> +	if(asprintf(&cmd, "%s %s", "set_sock_path", sock_path) == -1) {
> +		ERR("ustcmd_set_sock_path : asprintf failed (set_sock_path, %s",
> +				sock_path);
> +		return -1;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>  	if (result != 1) {
> @@ -456,7 +480,10 @@ int ustcmd_get_sock_path(char **sock_path, pid_t pid)
>  	char *cmd, *reply;
>  	int result;
>  
> -	asprintf(&cmd, "%s", "get_sock_path");
> +	if(asprintf(&cmd, "%s", "get_sock_path") == -1) {
> +		ERR("ustcmd_get_sock_path : asprintf failed");
> +		return USTCMD_ERR_GEN;
> +	}
>  
>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>  	if (result != 1) {
> diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
> index f2b7a03..30c8f24 100644
> --- a/libustcomm/ustcomm.c
> +++ b/libustcomm/ustcomm.c
> @@ -163,7 +163,11 @@ int ustcomm_request_consumer(pid_t pid, const char *channel)
>  		return -1;
>  	}
>  
> -	asprintf(&msg, "collect %d %s", pid, channel); 
> +	if(asprintf(&msg, "collect %d %s", pid, channel) == -1) {
> +		ERR("ustcomm_request_consumer : asprintf failed (collect %d/%s)",
> +				pid, channel);
> +		return -1;
> +	}
>  
>  	/* don't signal it because it's the daemon */
>  	result = ustcomm_connect_path(path, &conn, -1);
> @@ -706,7 +710,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
>  	int retval = 0;
>  
>  	if(sock_path) {
> -		asprintf(&name, "%s", sock_path);
> +		if(asprintf(&name, "%s", sock_path) == -1) {
> +			ERR("ustcomm_init_ustd : asprintf failed (sock_path %s)",
> +					sock_path);
> +			return -1;
> +		}
>  	}
>  	else {
>  		int result;
> @@ -718,7 +726,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
>  			return -1;
>  		}
>  
> -		asprintf(&name, "%s/%s", SOCK_DIR, "ustd");
> +		if(asprintf(&name, "%s/%s", SOCK_DIR, "ustd") == -1) {
> +			ERR("ustcomm_init_ustd : asprintf failed (%s/ustd)",
> +					SOCK_DIR);
> +			return -1;
> +		}
>  	}
>  
>  	handle->server.listen_fd = init_named_socket(name, &handle->server.socketpath);
> @@ -864,7 +876,11 @@ char *nth_token(const char *str, int tok_no)
>  		retval = NULL;
>  	}
>  
> -	asprintf(&retval, "%.*s", (int)(end-start), start);
> +	if(asprintf(&retval, "%.*s", (int)(end-start), start) == -1) {
> +		ERR("nth_token : asprintf failed (%.*s)",
> +				(int)(end-start), start);
> +		return NULL;
> +	}
>  
>  	return retval;
>  }
> diff --git a/libustd/libustd.c b/libustd/libustd.c
> index 4935131..cc88cde 100644
> --- a/libustd/libustd.c
> +++ b/libustd/libustd.c
> @@ -58,7 +58,12 @@ int get_subbuffer(struct buffer_info *buf)
>  	int retval;
>  	int result;
>  
> -	asprintf(&send_msg, "get_subbuffer %s", buf->name);
> +	if(asprintf(&send_msg, "get_subbuffer %s", buf->name) == -1) {
> +		ERR("get_subbuffer : asprintf failed (%s)",
> +				buf->name);
> +		retval = -1;
> +		goto end;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	if((result == -1 && (errno == ECONNRESET || errno == EPIPE)) || result == 0) {
>  		DBG("app died while being traced");
> @@ -118,7 +123,12 @@ int put_subbuffer(struct buffer_info *buf)
>  	int retval;
>  	int result;
>  
> -	asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old);
> +	if(asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old) == -1) {
> +		ERR("put_subbuffer : asprintf failed (%s %ld)",
> +				buf->name, buf->consumed_old);
> +		retval = -1;
> +		goto end;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	if(result < 0 && (errno == ECONNRESET || errno == EPIPE)) {
>  		retval = PUT_SUBBUF_DIED;
> @@ -215,7 +225,10 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>  	}
>  
>  	/* get pidunique */
> -	asprintf(&send_msg, "get_pidunique");
> +	if(asprintf(&send_msg, "get_pidunique") == -1) {
> +		ERR("connect_buffer : asprintf failed (get_pidunique)");
> +		return NULL;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	free(send_msg);
>  	if(result == -1) {
> @@ -235,7 +248,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>  	DBG("got pidunique %lld", buf->pidunique);
>  
>  	/* get shmid */
> -	asprintf(&send_msg, "get_shmid %s", buf->name);
> +	if(asprintf(&send_msg, "get_shmid %s", buf->name) == -1) {
> +		ERR("connect_buffer : asprintf failed (get_schmid %s)",
> +				buf->name);
> +		return NULL;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	free(send_msg);
>  	if(result == -1) {
> @@ -255,7 +272,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>  	DBG("got shmids %d %d", buf->shmid, buf->bufstruct_shmid);
>  
>  	/* get n_subbufs */
> -	asprintf(&send_msg, "get_n_subbufs %s", buf->name);
> +	if(asprintf(&send_msg, "get_n_subbufs %s", buf->name) == -1) {
> +		ERR("connect_buffer : asprintf failed (get_n_subbufs %s)",
> +				buf->name);
> +		return NULL;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	free(send_msg);
>  	if(result == -1) {
> @@ -275,7 +296,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>  	DBG("got n_subbufs %d", buf->n_subbufs);
>  
>  	/* get subbuf size */
> -	asprintf(&send_msg, "get_subbuf_size %s", buf->name);
> +	if(asprintf(&send_msg, "get_subbuf_size %s", buf->name) == -1) {
> +		ERR("connect_buffer : asprintf failed (get_subbuf_size %s)",
> +				buf->name);
> +		return NULL;
> +	}
>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>  	free(send_msg);
>  	if(result == -1) {
> diff --git a/tests/hello/hello.c b/tests/hello/hello.c
> index 8147860..f58706b 100644
> --- a/tests/hello/hello.c
> +++ b/tests/hello/hello.c
> @@ -77,13 +77,15 @@ int main()
>  		usleep(100000);
>  	}
>  
> -	scanf("%*s");
> +	if(scanf("%*s") == EOF)
> +		PERROR("scanf failed");
>  
>  	ltt_trace_stop("auto");
>  	ltt_trace_destroy("auto", 0);
>  
>  	DBG("TRACE STOPPED");
> -	scanf("%*s");
> +	if(scanf("%*s") == EOF)
> +		PERROR("scanf failed");
>  
>  	return 0;
>  }
> diff --git a/ustd/ustd.c b/ustd/ustd.c
> index c0f4c59..f15c31a 100644
> --- a/ustd/ustd.c
> +++ b/ustd/ustd.c
> @@ -191,7 +191,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
>  		trace_path = USTD_DEFAULT_TRACE_PATH;
>  	}
>  
> -	asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique);
> +	if(asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique) == -1) {
> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld)",
> +				trace_path, buf->pid, buf->pidunique);
> +		return 1;
> +	}
>  	result = create_dir_if_needed(tmp);
>  	if(result == -1) {
>  		ERR("could not create directory %s", tmp);
> @@ -200,7 +204,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
>  	}
>  	free(tmp);
>  
> -	asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name);
> +	if(asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name) == -1) {
> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld/%s)",
> +				trace_path, buf->pid, buf->pidunique, buf->name);
> +		return 1;
> +	}
>  	result = fd = open(tmp, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 00600);
>  	if(result == -1) {
>  		PERROR("open");
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[David Goulet]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 10-09-03 02:52 PM, Mathieu Desnoyers wrote:
> * David Goulet (david.goulet at polymtl.ca) wrote:
>> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
>> ---
>>  libust/tracectl.c    |   63 ++++++++++++++++++++++++++++++++++++++++++--------
>>  libustcmd/ustcmd.c   |   41 +++++++++++++++++++++++++++-----
>>  libustcomm/ustcomm.c |   24 ++++++++++++++++---
>>  libustd/libustd.c    |   37 ++++++++++++++++++++++++----
>>  tests/hello/hello.c  |    6 +++-
>>  ustd/ustd.c          |   12 ++++++++-
>>  6 files changed, 152 insertions(+), 31 deletions(-)
>>
>> diff --git a/libust/tracectl.c b/libust/tracectl.c
>> index ac551d5..1bb841f 100644
>> --- a/libust/tracectl.c
>> +++ b/libust/tracectl.c
>> @@ -156,7 +156,11 @@ static void inform_consumer_daemon(const char *trace_name)
>>  			/* iterate on all cpus */
>>  			for(j=0; j<trace->channels[i].n_cpus; j++) {
>>  				char *buf;
>> -				asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j);
>> +				if(asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j) == -1) {
> 
> please standardize on the kernel coding style:
> 
> if ()
> for ()
> switch ()
> 

There is not a single if/for/switch in these files that follow this standard...
You still want me to change all if(asprin... to if (asprintf.. ?

> but function() and macroname().
> 
> Also, please use a if (asprintf(...) < 0) test. I know the manpage
> states that -1 is explicitely returned, but it's very typical for all
> negative values to be more explicit error value variants. So let's make
> these tests as similar as possible.

Fixed

David
> 
> Thanks,
> 
> Mathieu
> 
>> +					ERR("inform_consumer_daemon : asprintf failed (%s_%d)",
>> +							trace->channels[i].channel_name, j);
>> +					goto finish;
>> +				}
>>  				result = ustcomm_request_consumer(pid, buf);
>>  				if(result == -1) {
>>  					WARN("Failed to request collection for channel %s. Is the daemon available?", trace->channels[i].channel_name);
>> @@ -213,7 +217,11 @@ int process_blkd_consumer_act(void *priv, int fd, short events)
>>  	else if(result < 0) {
>>  		ERR("ust_buffers_get_subbuf: error: %s", strerror(-result));
>>  	}
>> -	asprintf(&reply, "%s %ld", "OK", consumed_old);
>> +	if(asprintf(&reply, "%s %ld", "OK", consumed_old) == -1) {
>> +		ERR("process_blkd_consumer_act : asprintf failed (OK %ld)",
>> +				consumed_old);
>> +		return -1;
>> +	}
>>  	result = ustcomm_send_reply(&bc->server, reply, &bc->src);
>>  	if(result < 0) {
>>  		ERR("ustcomm_send_reply failed");
>> @@ -250,7 +258,11 @@ void seperate_channel_cpu(const char *channel_and_cpu, char **channel, int *cpu)
>>  		*cpu = atoi(sep+1);
>>  	}
>>  
>> -	asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu);
>> +	if(asprintf(channel, "%.*s", (int)(sep-channel_and_cpu), channel_and_cpu) == -1) {
>> +		ERR("seperate_channel_cpu : asprintf failed (%.*s)",
>> +				(int)(sep-channel_and_cpu), channel_and_cpu);
>> +		return;
>> +	}
>>  }
>>  
>>  static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
>> @@ -298,7 +310,12 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
>>  
>>  //			DBG("the shmid for the requested channel is %d", buf->shmid);
>>  //			DBG("the shmid for its buffer structure is %d", channel->buf_struct_shmids);
>> -			asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]);
>> +			if(asprintf(&reply, "%d %d", buf->shmid, channel->buf_struct_shmids[ch_cpu]) == -1) {
>> +				ERR("do_cmd_get_shmid : asprintf failed (%d %d)",
>> +						buf->shmid, channel->buf_struct_shmids[ch_cpu]);
>> +				retval = -1;
>> +				goto free_short_chan_name;
>> +			}
>>  
>>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>>  			if(result) {
>> @@ -369,7 +386,12 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
>>  			char *reply;
>>  
>>  			DBG("the n_subbufs for the requested channel is %d", channel->subbuf_cnt);
>> -			asprintf(&reply, "%d", channel->subbuf_cnt);
>> +			if(asprintf(&reply, "%d", channel->subbuf_cnt) == -1) {
>> +				ERR("do_cmd_get_n_subbufs : asprintf failed (%d)",
>> +						channel->subbuf_cnt);
>> +				retval = -1;
>> +				goto free_short_chan_name;
>> +			}
>>  
>>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>>  			if(result) {
>> @@ -438,7 +460,12 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>>  			char *reply;
>>  
>>  			DBG("the subbuf_size for the requested channel is %zd", channel->subbuf_size);
>> -			asprintf(&reply, "%zd", channel->subbuf_size);
>> +			if(asprintf(&reply, "%zd", channel->subbuf_size) == -1) {
>> +				ERR("do_cmd_get_subbuf_size : asprintf failed (%zd)",
>> +						channel->subbuf_size);
>> +				retval = -1;
>> +				goto free_short_chan_name;
>> +			}
>>  
>>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>>  			if(result) {
>> @@ -746,11 +773,19 @@ static int do_cmd_put_subbuffer(const char *recvbuf, struct ustcomm_source *src)
>>  			result = ust_buffers_put_subbuf(buf, consumed_old);
>>  			if(result < 0) {
>>  				WARN("ust_buffers_put_subbuf: error (subbuf=%s)", channel_and_cpu);
>> -				asprintf(&reply, "%s", "ERROR");
>> +				if(asprintf(&reply, "%s", "ERROR") == -1) {
>> +					ERR("do_cmd_put_subbuffer : asprintf failed (ERROR)");
>> +					retval = -1;
>> +					goto unlock_traces;
>> +				}
>>  			}
>>  			else {
>>  				DBG("ust_buffers_put_subbuf: success (subbuf=%s)", channel_and_cpu);
>> -				asprintf(&reply, "%s", "OK");
>> +				if(asprintf(&reply, "%s", "OK") == -1) {
>> +					ERR("do_cmd_put_subbuffer : asprintf failed (OK)");
>> +					retval = -1;
>> +					goto unlock_traces;
>> +				}
>>  			}
>>  
>>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>> @@ -992,7 +1027,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
>>  	else if(nth_token_is(recvbuf, "get_pidunique", 0) == 1) {
>>  		char *reply;
>>  
>> -		asprintf(&reply, "%lld", pidunique);
>> +		if(asprintf(&reply, "%lld", pidunique) == -1) {
>> +			ERR("process_client_cmd : asprintf failed (%lld)",
>> +					pidunique);
>> +			goto next_cmd;
>> +		}
>>  
>>  		result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>>  		if(result) {
>> @@ -1005,7 +1044,11 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src)
>>  	else if(nth_token_is(recvbuf, "get_sock_path", 0) == 1) {
>>  		char *reply = getenv("UST_DAEMON_SOCKET");
>>  		if(!reply) {
>> -			asprintf(&reply, "%s/%s", SOCK_DIR, "ustd");
>> +			if(asprintf(&reply, "%s/%s", SOCK_DIR, "ustd") == -1) {
>> +				ERR("process_client_cmd : asprintf failed (%s/ustd)",
>> +						SOCK_DIR);
>> +				goto next_cmd;
>> +			}
>>  			result = ustcomm_send_reply(&ustcomm_app.server, reply, src);
>>  			free(reply);
>>  		}
>> diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c
>> index cf6b9d7..1d35288 100644
>> --- a/libustcmd/ustcmd.c
>> +++ b/libustcmd/ustcmd.c
>> @@ -90,7 +90,11 @@ int ustcmd_set_marker_state(const char *mn, int state, pid_t pid)
>>  		return USTCMD_ERR_ARG;
>>  	}
>>  
>> -	asprintf(&cmd, "%s %s", cmd_str[state], mn);
>> +	if(asprintf(&cmd, "%s %s", cmd_str[state], mn) == -1) {
>> +		ERR("ustcmd_set_marker_state : asprintf failed (%s %s)",
>> +				cmd_str[state], mn);
>> +		return USTCMD_ERR_GEN;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>>  	if (result) {
>> @@ -114,7 +118,11 @@ int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
>>  	char *cmd;
>>  	int result;
>>  
>> -	asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size);
>> +	if(asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size) == -1) {
>> +		ERR("ustcmd_set_subbuf_size : asprintf failed (set_subbuf_size %s)",
>> +				channel_size);
>> +		return -1;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>>  	if (result != 1) {
>> @@ -138,7 +146,11 @@ int ustcmd_set_subbuf_num(const char *channel_size, pid_t pid)
>>  	char *cmd;
>>  	int result;
>>  
>> -	asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size);
>> +	if(asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size) == -1) {
>> +		ERR("ustcmd_set_subbuf_num : asprintf failed (set_subbuf_num %s",
>> +				channel_size);
>> +		return -1;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>>  	if (result != 1) {
>> @@ -163,7 +175,11 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
>>  	int result;
>>  
>>  	/* format: channel_cpu */
>> -	asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel);
>> +	if(asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel) == -1) {
>> +		ERR("ustcmd_get_subbuf_size : asprintf failed (get_subbuf_size, %s_0",
>> +				channel);
>> +		return -1;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>>  	if (result) {
>> @@ -191,7 +207,11 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
>>  	int result;
>>  
>>  	/* format: channel_cpu */
>> -	asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel);
>> +	if(asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel) == -1) {
>> +		ERR("ustcmd_get_subbuf_num : asprintf failed (get_n_subbufs, %s_0",
>> +				channel);
>> +		return -1;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>>  	if (result) {
>> @@ -432,7 +452,11 @@ int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
>>  	char *cmd;
>>  	int result;
>>  
>> -	asprintf(&cmd, "%s %s", "set_sock_path", sock_path);
>> +	if(asprintf(&cmd, "%s %s", "set_sock_path", sock_path) == -1) {
>> +		ERR("ustcmd_set_sock_path : asprintf failed (set_sock_path, %s",
>> +				sock_path);
>> +		return -1;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, NULL);
>>  	if (result != 1) {
>> @@ -456,7 +480,10 @@ int ustcmd_get_sock_path(char **sock_path, pid_t pid)
>>  	char *cmd, *reply;
>>  	int result;
>>  
>> -	asprintf(&cmd, "%s", "get_sock_path");
>> +	if(asprintf(&cmd, "%s", "get_sock_path") == -1) {
>> +		ERR("ustcmd_get_sock_path : asprintf failed");
>> +		return USTCMD_ERR_GEN;
>> +	}
>>  
>>  	result = ustcmd_send_cmd(cmd, pid, &reply);
>>  	if (result != 1) {
>> diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
>> index f2b7a03..30c8f24 100644
>> --- a/libustcomm/ustcomm.c
>> +++ b/libustcomm/ustcomm.c
>> @@ -163,7 +163,11 @@ int ustcomm_request_consumer(pid_t pid, const char *channel)
>>  		return -1;
>>  	}
>>  
>> -	asprintf(&msg, "collect %d %s", pid, channel); 
>> +	if(asprintf(&msg, "collect %d %s", pid, channel) == -1) {
>> +		ERR("ustcomm_request_consumer : asprintf failed (collect %d/%s)",
>> +				pid, channel);
>> +		return -1;
>> +	}
>>  
>>  	/* don't signal it because it's the daemon */
>>  	result = ustcomm_connect_path(path, &conn, -1);
>> @@ -706,7 +710,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
>>  	int retval = 0;
>>  
>>  	if(sock_path) {
>> -		asprintf(&name, "%s", sock_path);
>> +		if(asprintf(&name, "%s", sock_path) == -1) {
>> +			ERR("ustcomm_init_ustd : asprintf failed (sock_path %s)",
>> +					sock_path);
>> +			return -1;
>> +		}
>>  	}
>>  	else {
>>  		int result;
>> @@ -718,7 +726,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
>>  			return -1;
>>  		}
>>  
>> -		asprintf(&name, "%s/%s", SOCK_DIR, "ustd");
>> +		if(asprintf(&name, "%s/%s", SOCK_DIR, "ustd") == -1) {
>> +			ERR("ustcomm_init_ustd : asprintf failed (%s/ustd)",
>> +					SOCK_DIR);
>> +			return -1;
>> +		}
>>  	}
>>  
>>  	handle->server.listen_fd = init_named_socket(name, &handle->server.socketpath);
>> @@ -864,7 +876,11 @@ char *nth_token(const char *str, int tok_no)
>>  		retval = NULL;
>>  	}
>>  
>> -	asprintf(&retval, "%.*s", (int)(end-start), start);
>> +	if(asprintf(&retval, "%.*s", (int)(end-start), start) == -1) {
>> +		ERR("nth_token : asprintf failed (%.*s)",
>> +				(int)(end-start), start);
>> +		return NULL;
>> +	}
>>  
>>  	return retval;
>>  }
>> diff --git a/libustd/libustd.c b/libustd/libustd.c
>> index 4935131..cc88cde 100644
>> --- a/libustd/libustd.c
>> +++ b/libustd/libustd.c
>> @@ -58,7 +58,12 @@ int get_subbuffer(struct buffer_info *buf)
>>  	int retval;
>>  	int result;
>>  
>> -	asprintf(&send_msg, "get_subbuffer %s", buf->name);
>> +	if(asprintf(&send_msg, "get_subbuffer %s", buf->name) == -1) {
>> +		ERR("get_subbuffer : asprintf failed (%s)",
>> +				buf->name);
>> +		retval = -1;
>> +		goto end;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	if((result == -1 && (errno == ECONNRESET || errno == EPIPE)) || result == 0) {
>>  		DBG("app died while being traced");
>> @@ -118,7 +123,12 @@ int put_subbuffer(struct buffer_info *buf)
>>  	int retval;
>>  	int result;
>>  
>> -	asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old);
>> +	if(asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old) == -1) {
>> +		ERR("put_subbuffer : asprintf failed (%s %ld)",
>> +				buf->name, buf->consumed_old);
>> +		retval = -1;
>> +		goto end;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	if(result < 0 && (errno == ECONNRESET || errno == EPIPE)) {
>>  		retval = PUT_SUBBUF_DIED;
>> @@ -215,7 +225,10 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>>  	}
>>  
>>  	/* get pidunique */
>> -	asprintf(&send_msg, "get_pidunique");
>> +	if(asprintf(&send_msg, "get_pidunique") == -1) {
>> +		ERR("connect_buffer : asprintf failed (get_pidunique)");
>> +		return NULL;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	free(send_msg);
>>  	if(result == -1) {
>> @@ -235,7 +248,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>>  	DBG("got pidunique %lld", buf->pidunique);
>>  
>>  	/* get shmid */
>> -	asprintf(&send_msg, "get_shmid %s", buf->name);
>> +	if(asprintf(&send_msg, "get_shmid %s", buf->name) == -1) {
>> +		ERR("connect_buffer : asprintf failed (get_schmid %s)",
>> +				buf->name);
>> +		return NULL;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	free(send_msg);
>>  	if(result == -1) {
>> @@ -255,7 +272,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>>  	DBG("got shmids %d %d", buf->shmid, buf->bufstruct_shmid);
>>  
>>  	/* get n_subbufs */
>> -	asprintf(&send_msg, "get_n_subbufs %s", buf->name);
>> +	if(asprintf(&send_msg, "get_n_subbufs %s", buf->name) == -1) {
>> +		ERR("connect_buffer : asprintf failed (get_n_subbufs %s)",
>> +				buf->name);
>> +		return NULL;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	free(send_msg);
>>  	if(result == -1) {
>> @@ -275,7 +296,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
>>  	DBG("got n_subbufs %d", buf->n_subbufs);
>>  
>>  	/* get subbuf size */
>> -	asprintf(&send_msg, "get_subbuf_size %s", buf->name);
>> +	if(asprintf(&send_msg, "get_subbuf_size %s", buf->name) == -1) {
>> +		ERR("connect_buffer : asprintf failed (get_subbuf_size %s)",
>> +				buf->name);
>> +		return NULL;
>> +	}
>>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
>>  	free(send_msg);
>>  	if(result == -1) {
>> diff --git a/tests/hello/hello.c b/tests/hello/hello.c
>> index 8147860..f58706b 100644
>> --- a/tests/hello/hello.c
>> +++ b/tests/hello/hello.c
>> @@ -77,13 +77,15 @@ int main()
>>  		usleep(100000);
>>  	}
>>  
>> -	scanf("%*s");
>> +	if(scanf("%*s") == EOF)
>> +		PERROR("scanf failed");
>>  
>>  	ltt_trace_stop("auto");
>>  	ltt_trace_destroy("auto", 0);
>>  
>>  	DBG("TRACE STOPPED");
>> -	scanf("%*s");
>> +	if(scanf("%*s") == EOF)
>> +		PERROR("scanf failed");
>>  
>>  	return 0;
>>  }
>> diff --git a/ustd/ustd.c b/ustd/ustd.c
>> index c0f4c59..f15c31a 100644
>> --- a/ustd/ustd.c
>> +++ b/ustd/ustd.c
>> @@ -191,7 +191,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
>>  		trace_path = USTD_DEFAULT_TRACE_PATH;
>>  	}
>>  
>> -	asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique);
>> +	if(asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique) == -1) {
>> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld)",
>> +				trace_path, buf->pid, buf->pidunique);
>> +		return 1;
>> +	}
>>  	result = create_dir_if_needed(tmp);
>>  	if(result == -1) {
>>  		ERR("could not create directory %s", tmp);
>> @@ -200,7 +204,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
>>  	}
>>  	free(tmp);
>>  
>> -	asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name);
>> +	if(asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name) == -1) {
>> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld/%s)",
>> +				trace_path, buf->pid, buf->pidunique, buf->name);
>> +		return 1;
>> +	}
>>  	result = fd = open(tmp, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 00600);
>>  	if(result == -1) {
>>  		PERROR("open");
>> -- 
>> 1.7.0.4
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
> 

- -- 
David Goulet
LTTng project, DORSAL Lab.

1024D/16BD8563
BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkyBSigACgkQSvfBSxa9hWMnuACfWyswKn8S4v0m3FP6hvV/ag8Z
Xi8AnR4vCQuFX36IxqmmDMUqT4cdxHJI
=KMS+
-----END PGP SIGNATURE-----

[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[Mathieu Desnoyers]

* David Goulet (david.goulet at polymtl.ca) wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> 
> 
> On 10-09-03 02:52 PM, Mathieu Desnoyers wrote:
> > * David Goulet (david.goulet at polymtl.ca) wrote:
> >> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
> >> ---
> >>  libust/tracectl.c    |   63 ++++++++++++++++++++++++++++++++++++++++++--------
> >>  libustcmd/ustcmd.c   |   41 +++++++++++++++++++++++++++-----
> >>  libustcomm/ustcomm.c |   24 ++++++++++++++++---
> >>  libustd/libustd.c    |   37 ++++++++++++++++++++++++----
> >>  tests/hello/hello.c  |    6 +++-
> >>  ustd/ustd.c          |   12 ++++++++-
> >>  6 files changed, 152 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/libust/tracectl.c b/libust/tracectl.c
> >> index ac551d5..1bb841f 100644
> >> --- a/libust/tracectl.c
> >> +++ b/libust/tracectl.c
> >> @@ -156,7 +156,11 @@ static void inform_consumer_daemon(const char *trace_name)
> >>  			/* iterate on all cpus */
> >>  			for(j=0; j<trace->channels[i].n_cpus; j++) {
> >>  				char *buf;
> >> -				asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j);
> >> +				if(asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j) == -1) {
> > 
> > please standardize on the kernel coding style:
> > 
> > if ()
> > for ()
> > switch ()
> > 
> 
> There is not a single if/for/switch in these files that follow this standard...
> You still want me to change all if(asprin... to if (asprintf.. ?
> 
[...]
> >>  	if (result) {

there is at least one ;)

> >> @@ -114,7 +118,11 @@ int ustcmd_set_subbuf_size(const char *channel_size, pid_t pid)
> >>  	char *cmd;
> >>  	int result;
> >>  
> >> -	asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size);
> >> +	if(asprintf(&cmd, "%s %s", "set_subbuf_size", channel_size) == -1) {
> >> +		ERR("ustcmd_set_subbuf_size : asprintf failed (set_subbuf_size %s)",
> >> +				channel_size);
> >> +		return -1;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, NULL);
> >>  	if (result != 1) {

and two...

So yes.

Mathieu

> >> @@ -138,7 +146,11 @@ int ustcmd_set_subbuf_num(const char *channel_size, pid_t pid)
> >>  	char *cmd;
> >>  	int result;
> >>  
> >> -	asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size);
> >> +	if(asprintf(&cmd, "%s %s", "set_subbuf_num", channel_size) == -1) {
> >> +		ERR("ustcmd_set_subbuf_num : asprintf failed (set_subbuf_num %s",
> >> +				channel_size);
> >> +		return -1;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, NULL);
> >>  	if (result != 1) {
> >> @@ -163,7 +175,11 @@ int ustcmd_get_subbuf_size(const char *channel, pid_t pid)
> >>  	int result;
> >>  
> >>  	/* format: channel_cpu */
> >> -	asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel);
> >> +	if(asprintf(&cmd, "%s %s_0", "get_subbuf_size", channel) == -1) {
> >> +		ERR("ustcmd_get_subbuf_size : asprintf failed (get_subbuf_size, %s_0",
> >> +				channel);
> >> +		return -1;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, &reply);
> >>  	if (result) {
> >> @@ -191,7 +207,11 @@ int ustcmd_get_subbuf_num(const char *channel, pid_t pid)
> >>  	int result;
> >>  
> >>  	/* format: channel_cpu */
> >> -	asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel);
> >> +	if(asprintf(&cmd, "%s %s_0", "get_n_subbufs", channel) == -1) {
> >> +		ERR("ustcmd_get_subbuf_num : asprintf failed (get_n_subbufs, %s_0",
> >> +				channel);
> >> +		return -1;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, &reply);
> >>  	if (result) {
> >> @@ -432,7 +452,11 @@ int ustcmd_set_sock_path(const char *sock_path, pid_t pid)
> >>  	char *cmd;
> >>  	int result;
> >>  
> >> -	asprintf(&cmd, "%s %s", "set_sock_path", sock_path);
> >> +	if(asprintf(&cmd, "%s %s", "set_sock_path", sock_path) == -1) {
> >> +		ERR("ustcmd_set_sock_path : asprintf failed (set_sock_path, %s",
> >> +				sock_path);
> >> +		return -1;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, NULL);
> >>  	if (result != 1) {
> >> @@ -456,7 +480,10 @@ int ustcmd_get_sock_path(char **sock_path, pid_t pid)
> >>  	char *cmd, *reply;
> >>  	int result;
> >>  
> >> -	asprintf(&cmd, "%s", "get_sock_path");
> >> +	if(asprintf(&cmd, "%s", "get_sock_path") == -1) {
> >> +		ERR("ustcmd_get_sock_path : asprintf failed");
> >> +		return USTCMD_ERR_GEN;
> >> +	}
> >>  
> >>  	result = ustcmd_send_cmd(cmd, pid, &reply);
> >>  	if (result != 1) {
> >> diff --git a/libustcomm/ustcomm.c b/libustcomm/ustcomm.c
> >> index f2b7a03..30c8f24 100644
> >> --- a/libustcomm/ustcomm.c
> >> +++ b/libustcomm/ustcomm.c
> >> @@ -163,7 +163,11 @@ int ustcomm_request_consumer(pid_t pid, const char *channel)
> >>  		return -1;
> >>  	}
> >>  
> >> -	asprintf(&msg, "collect %d %s", pid, channel); 
> >> +	if(asprintf(&msg, "collect %d %s", pid, channel) == -1) {
> >> +		ERR("ustcomm_request_consumer : asprintf failed (collect %d/%s)",
> >> +				pid, channel);
> >> +		return -1;
> >> +	}
> >>  
> >>  	/* don't signal it because it's the daemon */
> >>  	result = ustcomm_connect_path(path, &conn, -1);
> >> @@ -706,7 +710,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
> >>  	int retval = 0;
> >>  
> >>  	if(sock_path) {
> >> -		asprintf(&name, "%s", sock_path);
> >> +		if(asprintf(&name, "%s", sock_path) == -1) {
> >> +			ERR("ustcomm_init_ustd : asprintf failed (sock_path %s)",
> >> +					sock_path);
> >> +			return -1;
> >> +		}
> >>  	}
> >>  	else {
> >>  		int result;
> >> @@ -718,7 +726,11 @@ int ustcomm_init_ustd(struct ustcomm_ustd *handle, const char *sock_path)
> >>  			return -1;
> >>  		}
> >>  
> >> -		asprintf(&name, "%s/%s", SOCK_DIR, "ustd");
> >> +		if(asprintf(&name, "%s/%s", SOCK_DIR, "ustd") == -1) {
> >> +			ERR("ustcomm_init_ustd : asprintf failed (%s/ustd)",
> >> +					SOCK_DIR);
> >> +			return -1;
> >> +		}
> >>  	}
> >>  
> >>  	handle->server.listen_fd = init_named_socket(name, &handle->server.socketpath);
> >> @@ -864,7 +876,11 @@ char *nth_token(const char *str, int tok_no)
> >>  		retval = NULL;
> >>  	}
> >>  
> >> -	asprintf(&retval, "%.*s", (int)(end-start), start);
> >> +	if(asprintf(&retval, "%.*s", (int)(end-start), start) == -1) {
> >> +		ERR("nth_token : asprintf failed (%.*s)",
> >> +				(int)(end-start), start);
> >> +		return NULL;
> >> +	}
> >>  
> >>  	return retval;
> >>  }
> >> diff --git a/libustd/libustd.c b/libustd/libustd.c
> >> index 4935131..cc88cde 100644
> >> --- a/libustd/libustd.c
> >> +++ b/libustd/libustd.c
> >> @@ -58,7 +58,12 @@ int get_subbuffer(struct buffer_info *buf)
> >>  	int retval;
> >>  	int result;
> >>  
> >> -	asprintf(&send_msg, "get_subbuffer %s", buf->name);
> >> +	if(asprintf(&send_msg, "get_subbuffer %s", buf->name) == -1) {
> >> +		ERR("get_subbuffer : asprintf failed (%s)",
> >> +				buf->name);
> >> +		retval = -1;
> >> +		goto end;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	if((result == -1 && (errno == ECONNRESET || errno == EPIPE)) || result == 0) {
> >>  		DBG("app died while being traced");
> >> @@ -118,7 +123,12 @@ int put_subbuffer(struct buffer_info *buf)
> >>  	int retval;
> >>  	int result;
> >>  
> >> -	asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old);
> >> +	if(asprintf(&send_msg, "put_subbuffer %s %ld", buf->name, buf->consumed_old) == -1) {
> >> +		ERR("put_subbuffer : asprintf failed (%s %ld)",
> >> +				buf->name, buf->consumed_old);
> >> +		retval = -1;
> >> +		goto end;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	if(result < 0 && (errno == ECONNRESET || errno == EPIPE)) {
> >>  		retval = PUT_SUBBUF_DIED;
> >> @@ -215,7 +225,10 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
> >>  	}
> >>  
> >>  	/* get pidunique */
> >> -	asprintf(&send_msg, "get_pidunique");
> >> +	if(asprintf(&send_msg, "get_pidunique") == -1) {
> >> +		ERR("connect_buffer : asprintf failed (get_pidunique)");
> >> +		return NULL;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	free(send_msg);
> >>  	if(result == -1) {
> >> @@ -235,7 +248,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
> >>  	DBG("got pidunique %lld", buf->pidunique);
> >>  
> >>  	/* get shmid */
> >> -	asprintf(&send_msg, "get_shmid %s", buf->name);
> >> +	if(asprintf(&send_msg, "get_shmid %s", buf->name) == -1) {
> >> +		ERR("connect_buffer : asprintf failed (get_schmid %s)",
> >> +				buf->name);
> >> +		return NULL;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	free(send_msg);
> >>  	if(result == -1) {
> >> @@ -255,7 +272,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
> >>  	DBG("got shmids %d %d", buf->shmid, buf->bufstruct_shmid);
> >>  
> >>  	/* get n_subbufs */
> >> -	asprintf(&send_msg, "get_n_subbufs %s", buf->name);
> >> +	if(asprintf(&send_msg, "get_n_subbufs %s", buf->name) == -1) {
> >> +		ERR("connect_buffer : asprintf failed (get_n_subbufs %s)",
> >> +				buf->name);
> >> +		return NULL;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	free(send_msg);
> >>  	if(result == -1) {
> >> @@ -275,7 +296,11 @@ struct buffer_info *connect_buffer(struct libustd_instance *instance, pid_t pid,
> >>  	DBG("got n_subbufs %d", buf->n_subbufs);
> >>  
> >>  	/* get subbuf size */
> >> -	asprintf(&send_msg, "get_subbuf_size %s", buf->name);
> >> +	if(asprintf(&send_msg, "get_subbuf_size %s", buf->name) == -1) {
> >> +		ERR("connect_buffer : asprintf failed (get_subbuf_size %s)",
> >> +				buf->name);
> >> +		return NULL;
> >> +	}
> >>  	result = ustcomm_send_request(buf->conn, send_msg, &received_msg);
> >>  	free(send_msg);
> >>  	if(result == -1) {
> >> diff --git a/tests/hello/hello.c b/tests/hello/hello.c
> >> index 8147860..f58706b 100644
> >> --- a/tests/hello/hello.c
> >> +++ b/tests/hello/hello.c
> >> @@ -77,13 +77,15 @@ int main()
> >>  		usleep(100000);
> >>  	}
> >>  
> >> -	scanf("%*s");
> >> +	if(scanf("%*s") == EOF)
> >> +		PERROR("scanf failed");
> >>  
> >>  	ltt_trace_stop("auto");
> >>  	ltt_trace_destroy("auto", 0);
> >>  
> >>  	DBG("TRACE STOPPED");
> >> -	scanf("%*s");
> >> +	if(scanf("%*s") == EOF)
> >> +		PERROR("scanf failed");
> >>  
> >>  	return 0;
> >>  }
> >> diff --git a/ustd/ustd.c b/ustd/ustd.c
> >> index c0f4c59..f15c31a 100644
> >> --- a/ustd/ustd.c
> >> +++ b/ustd/ustd.c
> >> @@ -191,7 +191,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
> >>  		trace_path = USTD_DEFAULT_TRACE_PATH;
> >>  	}
> >>  
> >> -	asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique);
> >> +	if(asprintf(&tmp, "%s/%u_%lld", trace_path, buf->pid, buf->pidunique) == -1) {
> >> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld)",
> >> +				trace_path, buf->pid, buf->pidunique);
> >> +		return 1;
> >> +	}
> >>  	result = create_dir_if_needed(tmp);
> >>  	if(result == -1) {
> >>  		ERR("could not create directory %s", tmp);
> >> @@ -200,7 +204,11 @@ int on_open_buffer(struct libustd_callbacks *data, struct buffer_info *buf)
> >>  	}
> >>  	free(tmp);
> >>  
> >> -	asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name);
> >> +	if(asprintf(&tmp, "%s/%u_%lld/%s", trace_path, buf->pid, buf->pidunique, buf->name) == -1) {
> >> +		ERR("on_open_buffer : asprintf failed (%s/%u_%lld/%s)",
> >> +				trace_path, buf->pid, buf->pidunique, buf->name);
> >> +		return 1;
> >> +	}
> >>  	result = fd = open(tmp, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 00600);
> >>  	if(result == -1) {
> >>  		PERROR("open");
> >> -- 
> >> 1.7.0.4
> >>
> >>
> >> _______________________________________________
> >> ltt-dev mailing list
> >> ltt-dev at lists.casi.polymtl.ca
> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >>
> > 
> 
> - -- 
> David Goulet
> LTTng project, DORSAL Lab.
> 
> 1024D/16BD8563
> BE3C 672B 9331 9796 291A  14C6 4AF7 C14B 16BD 8563
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
> 
> iEYEARECAAYFAkyBSigACgkQSvfBSxa9hWMnuACfWyswKn8S4v0m3FP6hvV/ag8Z
> Xi8AnR4vCQuFX36IxqmmDMUqT4cdxHJI
> =KMS+
> -----END PGP SIGNATURE-----
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH 2/2] Fix retval value assignment on error

[Nils Carlson]

Applied, thanks.

/Nils

On Fri, 3 Sep 2010, Mathieu Desnoyers wrote:

> * David Goulet (david.goulet at polymtl.ca) wrote:
>> Signed-off-by: David Goulet <david.goulet at polymtl.ca>
>
> Acked-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>
>> ---
>>  libust/tracectl.c |   11 +++++++++++
>>  1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/libust/tracectl.c b/libust/tracectl.c
>> index 1bb841f..909cbe8 100644
>> --- a/libust/tracectl.c
>> +++ b/libust/tracectl.c
>> @@ -282,12 +282,14 @@ static int do_cmd_get_shmid(const char *recvbuf, struct ustcomm_source *src)
>>  	channel_and_cpu = nth_token(recvbuf, 1);
>>  	if(channel_and_cpu == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>>  	if(ch_cpu == -1) {
>>  		ERR("problem parsing channel name");
>> +		retval = -1;
>>  		goto free_short_chan_name;
>>  	}
>>
>> @@ -360,12 +362,14 @@ static int do_cmd_get_n_subbufs(const char *recvbuf, struct ustcomm_source *src)
>>  	channel_and_cpu = nth_token(recvbuf, 1);
>>  	if(channel_and_cpu == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>>  	if(ch_cpu == -1) {
>>  		ERR("problem parsing channel name");
>> +		retval = -1;
>>  		goto free_short_chan_name;
>>  	}
>>
>> @@ -434,12 +438,14 @@ static int do_cmd_get_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>>  	channel_and_cpu = nth_token(recvbuf, 1);
>>  	if(channel_and_cpu == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>>  	if(ch_cpu == -1) {
>>  		ERR("problem parsing channel name");
>> +		retval = -1;
>>  		goto free_short_chan_name;
>>  	}
>>
>> @@ -522,6 +528,7 @@ static int do_cmd_set_subbuf_size(const char *recvbuf, struct ustcomm_source *sr
>>
>>  	if(ch_name == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>> @@ -577,10 +584,12 @@ static int do_cmd_set_subbuf_num(const char *recvbuf, struct ustcomm_source *src
>>
>>  	if(ch_name == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>  	if (num < 2) {
>>  		ERR("subbuffer count should be greater than 2");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>> @@ -629,12 +638,14 @@ static int do_cmd_get_subbuffer(const char *recvbuf, struct ustcomm_source *src)
>>  	channel_and_cpu = nth_token(recvbuf, 1);
>>  	if(channel_and_cpu == NULL) {
>>  		ERR("cannot parse channel");
>> +		retval = -1;
>>  		goto end;
>>  	}
>>
>>  	seperate_channel_cpu(channel_and_cpu, &ch_name, &ch_cpu);
>>  	if(ch_cpu == -1) {
>>  		ERR("problem parsing channel name");
>> +		retval = -1;
>>  		goto free_short_chan_name;
>>  	}
>>
>> --
>> 1.7.0.4
>>
>>
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>>
>
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>

[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value

[Pierre-Marc Fournier]

On 09/03/2010 02:52 PM, Mathieu Desnoyers wrote:
> * David Goulet (david.goulet at polymtl.ca) wrote:
>> Signed-off-by: David Goulet<david.goulet at polymtl.ca>
>> ---
>>   libust/tracectl.c    |   63 ++++++++++++++++++++++++++++++++++++++++++--------
>>   libustcmd/ustcmd.c   |   41 +++++++++++++++++++++++++++-----
>>   libustcomm/ustcomm.c |   24 ++++++++++++++++---
>>   libustd/libustd.c    |   37 ++++++++++++++++++++++++----
>>   tests/hello/hello.c  |    6 +++-
>>   ustd/ustd.c          |   12 ++++++++-
>>   6 files changed, 152 insertions(+), 31 deletions(-)
>>
>> diff --git a/libust/tracectl.c b/libust/tracectl.c
>> index ac551d5..1bb841f 100644
>> --- a/libust/tracectl.c
>> +++ b/libust/tracectl.c
>> @@ -156,7 +156,11 @@ static void inform_consumer_daemon(const char *trace_name)
>>   			/* iterate on all cpus */
>>   			for(j=0; j<trace->channels[i].n_cpus; j++) {
>>   				char *buf;
>> -				asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j);
>> +				if(asprintf(&buf, "%s_%d", trace->channels[i].channel_name, j) == -1) {
>
> please standardize on the kernel coding style:
>
> if ()
> for ()
> switch ()
>
> but function() and macroname().
>
> Also, please use a if (asprintf(...)<  0) test. I know the manpage
> states that -1 is explicitely returned, but it's very typical for all
> negative values to be more explicit error value variants. So let's make
> these tests as similar as possible.
>

No. It's typical in the linux kernel, but not in unix userspace and not 
in ust internal apis.

pmf