From mboxrd@z Thu Jan 1 00:00:00 1970 From: david.goulet@polymtl.ca (David Goulet) Date: Wed, 08 Sep 2010 14:23:30 -0400 Subject: [ltt-dev] [UST PATCH 3/3] Add functions and command line for listing trace_events In-Reply-To: <1283968341-12201-3-git-send-email-nils.carlson@ericsson.com> References: <1283968341-12201-1-git-send-email-nils.carlson@ericsson.com> <1283968341-12201-3-git-send-email-nils.carlson@ericsson.com> Message-ID: <4C87D4A2.10101@polymtl.ca> Comment below. On 10-09-08 01:52 PM, Nils Carlson wrote: > > Signed-off-by: Nils Carlson > --- > include/ust/ustcmd.h | 6 ++++ > libust/tracectl.c | 34 ++++++++++++++++++++- > libustcmd/ustcmd.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++ > ustctl/ustctl.c | 23 ++++++++++++++- > 4 files changed, 138 insertions(+), 3 deletions(-) > > diff --git a/include/ust/ustcmd.h b/include/ust/ustcmd.h > index 60f5018..986ae61 100644 > --- a/include/ust/ustcmd.h > +++ b/include/ust/ustcmd.h > @@ -43,6 +43,10 @@ struct marker_status { > char *fs; /* Format string (end of marker_status array if NULL) */ > }; > > +struct trace_event_status { > + char *name; > +}; > + > extern pid_t *ustcmd_get_online_pids(void); > extern int ustcmd_set_marker_state(const char *, int, pid_t); > extern int ustcmd_set_subbuf_size(const char *, pid_t); > @@ -59,6 +63,8 @@ extern int ustcmd_free_cmsf(struct marker_status *); > extern unsigned int ustcmd_count_nl(const char *); > extern int ustcmd_send_cmd(const char *, pid_t, char **); > extern int ustcmd_get_cmsf(struct marker_status **, pid_t); > +extern int ustcmd_free_tes(struct trace_event_status *); > +extern int ustcmd_get_tes(struct trace_event_status **, pid_t); > extern int ustcmd_set_sock_path(const char *, pid_t); > extern int ustcmd_get_sock_path(char **, pid_t); > extern int ustcmd_force_switch(pid_t); > diff --git a/libust/tracectl.c b/libust/tracectl.c > index e64b26f..7dbc572 100644 > --- a/libust/tracectl.c > +++ b/libust/tracectl.c > @@ -35,6 +35,7 @@ > #include > > #include > +#include > #include > #include "tracer.h" > #include "usterr.h" > @@ -127,6 +128,21 @@ static void print_markers(FILE *fp) > unlock_markers(); > } > > +static void print_trace_events(FILE *fp) > +{ > + struct trace_event_iter iter; > + > + lock_trace_events(); > + trace_event_iter_reset(&iter); > + trace_event_iter_start(&iter); > + > + while(iter.trace_event) { > + fprintf(fp, "trace_event: %s\n", iter.trace_event->name); > + trace_event_iter_next(&iter); > + } > + unlock_trace_events(); > +} > + > static int init_socket(void); > > /* Ask the daemon to collect a trace called trace_name and being > @@ -867,8 +883,22 @@ int process_client_cmd(char *recvbuf, struct ustcomm_source *src) > result = ustcomm_send_reply(&ustcomm_app.server, ptr, src); > > free(ptr); > - } > - else if(!strcmp(recvbuf, "start")) { > + } else if (!strcmp(recvbuf, "print_trace_events")) { > + print_trace_events(stderr); > + > + } else if(!strcmp(recvbuf, "list_trace_events")) { > + char *ptr; > + size_t size; > + FILE *fp; > + > + fp = open_memstream(&ptr,&size); Might want to check if fp is not NULL since print_trace_events don't check it either. > + print_trace_events(fp); > + fclose(fp); > + > + result = ustcomm_send_reply(&ustcomm_app.server, ptr, src); > + Following the discussion on ltt-dev about error handling and trying to have consistency through the code, it might be good to handle the return code here and print the appropriate error. (ERR macro) > + free(ptr); > + } else if(!strcmp(recvbuf, "start")) { > /* start is an operation that setups the trace, allocates it and starts it */ > result = ltt_trace_setup(trace_name); > if(result< 0) { > diff --git a/libustcmd/ustcmd.c b/libustcmd/ustcmd.c > index 5b4fd02..21e4846 100644 > --- a/libustcmd/ustcmd.c > +++ b/libustcmd/ustcmd.c > @@ -440,6 +440,84 @@ int ustcmd_get_cmsf(struct marker_status **cmsf, const pid_t pid) > return 0; > } > > + > +/** > + * Frees a TES array. > + * > + * @param tes TES array to free > + * @return 0 if successful, or error USTCMD_ERR_ARG > + */ > +int ustcmd_free_tes(struct trace_event_status *tes) > +{ > + if (tes == NULL) { > + return USTCMD_ERR_ARG; > + } > + > + unsigned int i = 0; > + while (tes[i].name != NULL) { > + free(tes[i].name); > + ++i; > + } > + free(tes); > + > + return 0; > +} > + > +/** > + * Gets trace_events string for a given PID. > + * > + * @param tes Pointer to TES array to be filled (callee allocates, caller > + * frees with `ustcmd_free_tes') > + * @param pid Targeted PID > + * @return 0 if successful, or -1 on error > + */ > +int ustcmd_get_tes(struct trace_event_status **tes, > + const pid_t pid) > +{ > + char *big_str = NULL; > + int result; > + struct trace_event_status *tmp_tes = NULL; > + unsigned int i = 0, tes_ind = 0; > + > + if (tes == NULL) { > + return -1; > + } > + result = ustcmd_send_cmd("list_trace_events", pid,&big_str); For a matter of code beauty :), there is a space missing after "pid, " > + if (result != 1) { > + return -1; > + } > + > + if (result != 1) { > + ERR("error while getting trace_event list"); > + return -1; > + } This last if, isn't a bit useless ;) > + > + tmp_tes = (struct trace_event_status *) malloc(sizeof(struct trace_event_status) * > + (ustcmd_count_nl(big_str) + 1)); malloc -> zmalloc > + if (tmp_tes == NULL) { > + return -1; > + } > + > + /* Parse received reply string (format: "[chan]/[mark] [st] [fs]"): */ > + while (big_str[i] != '\0') { > + char state; > + > + sscanf(big_str + i, "trace_event: %a[^\n]", > + &tmp_tes[tes_ind].name); > + while (big_str[i] != '\n') { > + ++i; /* Go to next '\n' */ > + } > + ++i; /* Skip current pointed '\n' */ > + ++tes_ind; > + } > + tmp_tes[tes_ind].name = NULL; > + > + *tes = tmp_tes; > + > + free(big_str); > + return 0; > +} > + > /** > * Set socket path > * > diff --git a/ustctl/ustctl.c b/ustctl/ustctl.c > index d290975..9daa043 100644 > --- a/ustctl/ustctl.c > +++ b/ustctl/ustctl.c > @@ -32,6 +32,7 @@ enum command { > STOP_TRACE, > DESTROY_TRACE, > LIST_MARKERS, > + LIST_TRACE_EVENTS, > ENABLE_MARKER, > DISABLE_MARKER, > GET_ONLINE_PIDS, > @@ -73,6 +74,7 @@ Commands:\n\ > --enable-marker \"CHANNEL/MARKER\"\tEnable a marker\n\ > --disable-marker \"CHANNEL/MARKER\"\tDisable a marker\n\ > --list-markers\t\t\tList the markers of the process, their\n\t\t\t\t\t state and format string\n\ > + --list-trace-events\t\t\tList the trace-events of the process\n\ > --force-switch\t\t\tForce a subbuffer switch\n\ > \ > "); > @@ -94,6 +96,7 @@ int parse_opts_long(int argc, char **argv, struct ust_opts *opts) > { "stop-trace", 0, 0, STOP_TRACE }, > { "destroy-trace", 0, 0, DESTROY_TRACE }, > { "list-markers", 0, 0, LIST_MARKERS }, > + { "list-trace-events", 0, 0, LIST_TRACE_EVENTS}, > { "enable-marker", 1, 0, ENABLE_MARKER }, > { "disable-marker", 1, 0, DISABLE_MARKER }, > { "help", 0, 0, 'h' }, > @@ -215,6 +218,8 @@ int main(int argc, char *argv[]) > > pidit = opts.pids; > struct marker_status *cmsf = NULL; > + struct trace_event_status *tes = NULL; > + unsigned int i = 0; > > while(*pidit != -1) { > switch (opts.cmd) { > @@ -258,7 +263,6 @@ int main(int argc, char *argv[]) > " PID %u\n", (unsigned int) *pidit); > break; > } > - unsigned int i = 0; > while (cmsf[i].channel != NULL) { > printf("{PID: %u, channel/marker: %s/%s, " > "state: %u, fmt: %s}\n", > @@ -272,6 +276,23 @@ int main(int argc, char *argv[]) > ustcmd_free_cmsf(cmsf); > break; > > + case LIST_TRACE_EVENTS: > + tes = NULL; Isn't set to NULL before. Why again? > + if (ustcmd_get_tes(&tes, *pidit)) { > + fprintf(stderr, > + "error while trying to list trace_events for" > + " PID %u\n", (unsigned int) *pidit); ERR macro do just that. We might want to use it everywhere instead of partial fprintf and ERR usage. Thanks David > + break; > + } > + while (tes[i].name != NULL) { > + printf("{PID: %u, trace_event: %s}\n", > + (unsigned int) *pidit, > + tes[i].name); > + ++i; > + } > + ustcmd_free_tes(tes); > + > + break; > case ENABLE_MARKER: > if(opts.regex) > ustcmd_set_marker_state(opts.regex, 1, *pidit); -- David Goulet LTTng project, DORSAL Lab. PGP/GPG : 1024D/16BD8563 BE3C 672B 9331 9796 291A 14C6 4AF7 C14B 16BD 8563