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


On 10-09-03 02:52 PM, Mathieu Desnoyers wrote:
> * David Goulet ([email protected]) wrote:
>> Signed-off-by: David Goulet <[email protected]>
>> ---
>>  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
>> [email protected]
>> 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 mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev

Reply via email to