[ltt-dev] [PATCH 1/2] Fix asprintf warning ignoring return value
Mathieu Desnoyers
compudj at krystal.dyndns.org
Fri Sep 3 14:52:24 EDT 2010
* 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
More information about the lttng-dev
mailing list