Hi Yannick, Thanks for looking into this. A few comments below,
* Yannick Brosseau ([email protected]) wrote: > Remove unused variable and some commented code > Use proper cast in depanalysis for the hash table entries > Fix other simple warnings > Also fix an unitialized variable usage detected by valgrind in depanalysis > > Signed-off-by: Yannick Brosseau <[email protected]> > --- > lttv/modules/text/batchAnalysis.c | 4 +- > lttv/modules/text/depanalysis.c | 107 > ++++++++++------------------------ > lttv/modules/text/precomputeState.c | 12 ---- > lttv/modules/text/sstack.h | 1 + > lttv/modules/text/sync_chain_batch.c | 2 +- > 5 files changed, 34 insertions(+), 92 deletions(-) > > diff --git a/lttv/modules/text/batchAnalysis.c > b/lttv/modules/text/batchAnalysis.c > index 4b02f33..a5f40c6 100644 > --- a/lttv/modules/text/batchAnalysis.c > +++ b/lttv/modules/text/batchAnalysis.c > @@ -68,7 +68,7 @@ static gboolean process_traceset(void *hook_data, void > *call_data) > > LttvIAttribute *attributes = LTTV_IATTRIBUTE(lttv_global_attributes()); > > - LttvTracesetStats *tscs; > + LttvTracesetStats *tscs = 0; = NULL. > > LttvTracesetState *tss; > > @@ -93,7 +93,7 @@ static gboolean process_traceset(void *hook_data, void > *call_data) > > syncTraceset(tc); > > - lttv_state_add_event_hooks(tc); > + lttv_state_add_event_hooks(tss); > if(a_stats) lttv_stats_add_event_hooks(tscs); > > retval= lttv_iattribute_find_by_path(attributes, "filter/expression", > diff --git a/lttv/modules/text/depanalysis.c b/lttv/modules/text/depanalysis.c > index 5a4064a..d1713ce 100644 > --- a/lttv/modules/text/depanalysis.c > +++ b/lttv/modules/text/depanalysis.c > @@ -19,6 +19,8 @@ > #ifdef HAVE_CONFIG_H > #include <config.h> > #endif > +#define _GNU_SOURCE > +#include <stdio.h> Ah, yes, not having _GNU_SOURCE defined just for some of the headers can lead to problems (e.g. not having some prototypes available). Good catch! > > #include <lttv/lttv.h> > #include <lttv/option.h> > @@ -32,8 +34,7 @@ > #include <ltt/ltt.h> > #include <ltt/event.h> > #include <ltt/trace.h> > -#define _GNU_SOURCE > -#include <stdio.h> > + > #include <glib.h> > #include <stdlib.h> > > @@ -403,7 +404,6 @@ static void prepare_pop_item_commit_nocheck(struct > process *p, enum llev_state s > static void prepare_pop_item_commit(struct process *p, enum llev_state st, > LttTime t) > { > struct process_with_state *pwstate; > - struct sstack_item *item = sstack_item_new_pop(); Why ? > > int push_idx; > > @@ -504,14 +504,8 @@ static GString *a_string; > > static gboolean write_traceset_header(void *hook_data, void *call_data) > { > - LttvTracesetContext *tc = (LttvTracesetContext *)call_data; > - > g_info("Traceset header"); > - > - /* Print the trace set header */ > - g_info(a_file,"Trace set contains %d traces\n\n", > - lttv_traceset_number(tc->ts)); > - > + > return FALSE; > } > > @@ -533,7 +527,7 @@ static void update_hlev_state(struct process *p, LttTime > t) > { > int i; > > - enum hlev_state new_hlev; > + enum hlev_state new_hlev = 0; > > for(i=p->stack_current; i>=0; i--) { > enum llev_state st; > @@ -609,6 +603,7 @@ static void update_hlev_state(struct process *p, LttTime > t) > /* init vals */ > hlev_blocked_private->syscall_id = 1; > hlev_blocked_private->trap = 0; > + hlev_blocked_private->pid_exit = 0; > hlev_blocked_private->substate = > HLEV_BLOCKED__UNDEFINED; > hlev_blocked_private->private = NULL; > hlev_blocked_private->llev_state_entry = > oldstyle_stack_to_garray(p->llev_state_stack, p->stack_current); > @@ -710,7 +705,7 @@ static void print_summary_item(struct summary_tree_node > *node, int depth) > GList *vals; > > if(depth >= 0) { > - printf("\t%*s (", strlen(node->name)+2*depth, node->name); > + printf("\t%*s (", (int)strlen(node->name)+2*depth, node->name); better with unsigned int. > print_time(node->duration); > printf(") <%d>\n", node->id_for_episodes); > } > @@ -734,19 +729,19 @@ static void print_summary_item(struct summary_tree_node > *node, int depth) > > static inline void print_irq(int irq) > { > - printf("IRQ %d [%s]", irq, > g_quark_to_string(g_hash_table_lookup(irq_table, &irq))); > + printf("IRQ %d [%s]", irq, g_quark_to_string((GQuark)(unsigned > long)g_hash_table_lookup(irq_table, &irq))); > } > > static inline void print_softirq(int softirq) > { > - printf("SoftIRQ %d [%s]", softirq, > g_quark_to_string(g_hash_table_lookup(softirq_table, &softirq))); > + printf("SoftIRQ %d [%s]", softirq, g_quark_to_string((GQuark)(unsigned > long)g_hash_table_lookup(softirq_table, &softirq))); > } > > static inline void print_pid(int pid) > { > struct process *event_process_info = > g_hash_table_lookup(process_hash_table, &pid); > > - char *pname; > + const char *pname; > > if(event_process_info == NULL) > pname = "?"; > @@ -757,17 +752,16 @@ static inline void print_pid(int pid) > > static void modify_path_with_private(GArray *path, struct process_state > *pstate) > { > - //GString tmps = g_string_new(""); > char *tmps; > > // FIXME: fix this leak > switch(pstate->bstate) { > case HLEV_INTERRUPTED_IRQ: > - asprintf(&tmps, "IRQ %d [%s]", ((struct > hlev_state_info_interrupted_irq *)pstate->private)->irq, > g_quark_to_string(g_hash_table_lookup(irq_table, &((struct > hlev_state_info_interrupted_irq *)pstate->private)->irq))); > + asprintf(&tmps, "IRQ %d [%s]", ((struct > hlev_state_info_interrupted_irq *)pstate->private)->irq, > g_quark_to_string((GQuark)(unsigned long)g_hash_table_lookup(irq_table, > &((struct hlev_state_info_interrupted_irq *)pstate->private)->irq))); > g_array_append_val(path, tmps); > break; > case HLEV_INTERRUPTED_SOFTIRQ: > - asprintf(&tmps, "SoftIRQ %d [%s]", ((struct > hlev_state_info_interrupted_softirq *)pstate->private)->softirq, > g_quark_to_string(g_hash_table_lookup(softirq_table, &((struct > hlev_state_info_interrupted_softirq *)pstate->private)->softirq))); > + asprintf(&tmps, "SoftIRQ %d [%s]", ((struct > hlev_state_info_interrupted_softirq *)pstate->private)->softirq, > g_quark_to_string((GQuark)(unsigned long)g_hash_table_lookup(softirq_table, > &((struct hlev_state_info_interrupted_softirq *)pstate->private)->softirq))); > g_array_append_val(path, tmps); > break; > case HLEV_BLOCKED: { > @@ -783,12 +777,12 @@ static void modify_path_with_private(GArray *path, > struct process_state *pstate) > g_array_append_val(path, ptr); > } > else { > - asprintf(&tmps, "Syscall %d [%s]", > hlev_blocked_private->syscall_id, > g_quark_to_string(g_hash_table_lookup(syscall_table, > &hlev_blocked_private->syscall_id))); > + asprintf(&tmps, "Syscall %d [%s]", > hlev_blocked_private->syscall_id, g_quark_to_string((GQuark)(unsigned > long)g_hash_table_lookup(syscall_table, &hlev_blocked_private->syscall_id))); > g_array_append_val(path, tmps); > } > > if(((struct hlev_state_info_blocked > *)pstate->private)->substate == HLEV_BLOCKED__OPEN) { > - char *str = g_quark_to_string(((struct > hlev_state_info_blocked__open *)((struct hlev_state_info_blocked > *)pstate->private)->private)->filename); > + const char *str = g_quark_to_string(((struct > hlev_state_info_blocked__open *)((struct hlev_state_info_blocked > *)pstate->private)->private)->filename); > g_array_append_val(path, str); > } > else if(((struct hlev_state_info_blocked > *)pstate->private)->substate == HLEV_BLOCKED__READ) { > @@ -823,7 +817,7 @@ void print_stack_garray_horizontal(GArray *stack) > > if(pstate->bstate == LLEV_SYSCALL) { > struct llev_state_info_syscall *llev_syscall_private = > pstate->private; > - printf(" %d [%s]", llev_syscall_private->syscall_id, > g_quark_to_string(g_hash_table_lookup(syscall_table, > &llev_syscall_private->syscall_id))); > + printf(" %d [%s]", llev_syscall_private->syscall_id, > g_quark_to_string((GQuark)(unsigned long)g_hash_table_lookup(syscall_table, > &llev_syscall_private->syscall_id))); > } > > printf(", "); > @@ -1032,7 +1026,6 @@ static void print_process_critical_path_summary() > { > struct process *pinfo; > GList *pinfos; > - int i,j; > > pinfos = g_hash_table_get_values(process_hash_table); > if(pinfos == NULL) { > @@ -1043,9 +1036,6 @@ static void print_process_critical_path_summary() > printf("Process Critical Path Summary:\n"); > > for(;;) { > - struct summary_tree_node base_node = { children: NULL }; > - > - struct process_state *hlev_state_cur; > > pinfo = (struct process *)pinfos->data; > if (depanalysis_range_pid_searching != -1 && pinfo->pid != > depanalysis_range_pid_searching) > @@ -1103,8 +1093,6 @@ static void print_simple_summary(void) > for(;;) { > struct summary_tree_node base_node = { children: NULL, name: > "Root" }; > > - struct process_state *hlev_state_cur; > - > pinfo = (struct process *)pinfos->data; > printf("\tProcess %d [%s]\n", pinfo->pid, > g_quark_to_string(pinfo->name)); > > @@ -1210,8 +1198,6 @@ static void print_simple_summary_pid_range(int pid, > LttTime t1, LttTime t2) > { > struct summary_tree_node base_node = { children: NULL, name: > "Root" }; > > - struct process_state *hlev_state_cur; > - > printf("\tProcess %d [%s]\n", pinfo->pid, > g_quark_to_string(pinfo->name)); > > /* For each state in the process history */ > @@ -1371,7 +1357,7 @@ void print_range_reports(int pid, LttTime t1, LttTime > t2) > else > iter_t2 = g_array_index(family, struct family_item, > i-1).creation; > > - printf("This section of summary concerns pid %d between "); > + printf("This section of summary concerns pid %d between > ",iter_pid); Please add a missing space (while we are here) : ", iter_pid".. > print_time(iter_t1); > printf(" and "); > print_time(iter_t2); > @@ -1383,16 +1369,7 @@ void print_range_reports(int pid, LttTime t1, LttTime > t2) > > static gboolean write_traceset_footer(void *hook_data, void *call_data) > { > - LttvTracesetContext *tc = (LttvTracesetContext *)call_data; > - > - g_info("TextDump traceset footer"); > - > - g_info(a_file,"End trace set\n\n"); > - > -// if(LTTV_IS_TRACESET_STATS(tc)) { > -// lttv_stats_sum_traceset((LttvTracesetStats *)tc, > ltt_time_infinite); > -// print_stats(a_file, (LttvTracesetStats *)tc); > -// } > + g_info("depanalysis traceset footer"); > > /* After processing all the events, we need to flush the sstacks > * because some unfinished states may remain in them. We want them > @@ -1418,28 +1395,9 @@ static gboolean write_traceset_footer(void *hook_data, > void *call_data) > return FALSE; > } > > -#if 0 > -static gboolean write_trace_header(void *hook_data, void *call_data) > -{ > - LttvTraceContext *tc = (LttvTraceContext *)call_data; > -#if 0 //FIXME > - LttSystemDescription *system = ltt_trace_system_description(tc->t); > - > - fprintf(a_file," Trace from %s in %s\n%s\n\n", > - ltt_trace_system_description_node_name(system), > - ltt_trace_system_description_domain_name(system), > - ltt_trace_system_description_description(system)); > -#endif //0 > - return FALSE; > -} > -#endif > - > > static int write_event_content(void *hook_data, void *call_data) > { > - gboolean result; > - > -// LttvIAttribute *attributes = LTTV_IATTRIBUTE(lttv_global_attributes()); > > LttvTracefileContext *tfc = (LttvTracefileContext *)call_data; > > @@ -1496,8 +1454,10 @@ static char *field_get_value_string(struct LttEvent > *e, struct marker_info *info > return ltt_event_get_string(e, marker_field); > } > > -void process_delayed_stack_action(struct process *pinfo, struct sstack_item > *item) > +void process_delayed_stack_action(void *arg, struct sstack_item *item) > { > + White line ? > + struct process *pinfo = (struct process *)arg; > //printf("processing delayed stack action on pid %d at ", pinfo->pid); > //if(((struct process_with_state *) > item->data_val)->state.time_begin.tv_nsec == 987799696) > // printf("HERE!!!\n"); > @@ -1550,7 +1510,8 @@ void process_delayed_stack_action(struct process > *pinfo, struct sstack_item *ite > > static struct process *get_or_init_process_info(struct LttEvent *e, GQuark > name, int pid, int *new) > { > - gconstpointer val; > + //gconstpointer val; > + gpointer val; > > val = g_hash_table_lookup(process_hash_table, &pid); > if(val == NULL) { > @@ -1610,7 +1571,7 @@ static int process_event(void *hook_data, void > *call_data) > guint cpu = tfs->cpu; > LttvTraceState *ts = (LttvTraceState*)tfc->t_context; > LttvProcessState *process = ts->running_process[cpu]; > - LttTrace *trace = ts->parent.t; > + //LttTrace *trace = ts->parent.t; > struct process *pinfo; > > e = ltt_tracefile_get_event(tfs->parent.tf); > @@ -1649,7 +1610,7 @@ static int process_event(void *hook_data, void > *call_data) > > *pint = field_get_value_int(e, info, LTT_FIELD_ID); > q = g_quark_from_string(field_get_value_string(e, info, > LTT_FIELD_SYMBOL)); > - g_hash_table_insert(syscall_table, pint, q); > + g_hash_table_insert(syscall_table, pint, (void*)(unsigned > long)q); With glib, please use "gpointer" rather than "void *". By the way, if you really want to keep the void * here (I'm not really opposed to it, as the code around uses "void *", please add a space between "void" and "*". > } > else if(tfc->tf->name == LTT_CHANNEL_IRQ_STATE && info->name == > LTT_EVENT_LIST_INTERRUPT) { > GQuark q; > @@ -1657,7 +1618,7 @@ static int process_event(void *hook_data, void > *call_data) > > *pint = field_get_value_int(e, info, LTT_FIELD_IRQ_ID); > q = g_quark_from_string(field_get_value_string(e, info, > LTT_FIELD_ACTION)); > - g_hash_table_insert(irq_table, pint, q); > + g_hash_table_insert(irq_table, pint, (void*)(unsigned long)q); > } > else if(tfc->tf->name == LTT_CHANNEL_SOFTIRQ_STATE && info->name == > LTT_EVENT_SOFTIRQ_VEC) { > GQuark q; > @@ -1665,7 +1626,7 @@ static int process_event(void *hook_data, void > *call_data) > > *pint = field_get_value_int(e, info, LTT_FIELD_ID); > q = g_quark_from_string(field_get_value_string(e, info, > LTT_FIELD_SYMBOL)); > - g_hash_table_insert(softirq_table, pint, q); > + g_hash_table_insert(softirq_table, pint, (void*)(unsigned > long)q); > } > > > @@ -1755,7 +1716,7 @@ static int process_event(void *hook_data, void > *call_data) > prepare_pop_item_commit(event_process_info, LLEV_SOFTIRQ, > e->event_time); > } > else if(tfc->tf->name == LTT_CHANNEL_KERNEL && info->name == > LTT_EVENT_PROCESS_FORK) { > - int pid = differentiate_swappers(field_get_value_int(e, info, > LTT_FIELD_CHILD_PID), e); > + //int pid = differentiate_swappers(field_get_value_int(e, info, > LTT_FIELD_CHILD_PID), e); What's the deal here ? Why don't we shrink the line below instead ? > struct process *event_process_info = > get_or_init_process_info(e, process->name, > differentiate_swappers(field_get_value_int(e, info, LTT_FIELD_CHILD_PID), e), > NULL); > struct sstack_item *item; > > @@ -1839,7 +1800,7 @@ static int process_event(void *hook_data, void > *call_data) > llev_syscall_read_private = llev_syscall_private->private; > > fd = field_get_value_int(e, info, LTT_FIELD_FD); > - pfileq = g_hash_table_lookup(process->fds, fd); > + pfileq = (GQuark)(unsigned > long)g_hash_table_lookup(process->fds, &fd); > if(pfileq) { > llev_syscall_read_private->filename = pfileq; > } > @@ -1885,7 +1846,7 @@ static int process_event(void *hook_data, void > *call_data) > llev_syscall_poll_private = llev_syscall_private->private; > > fd = field_get_value_int(e, info, LTT_FIELD_FD); > - pfileq = g_hash_table_lookup(process->fds, fd); > + pfileq = (GQuark)(unsigned > long)g_hash_table_lookup(process->fds, &fd); > if(pfileq) { > llev_syscall_poll_private->filename = pfileq; > } > @@ -1937,7 +1898,6 @@ static int process_event(void *hook_data, void > *call_data) > } > > next_iter: > - skip_state_machine: > return FALSE; > } > > @@ -2037,13 +1997,6 @@ static void init() > g_assert(event_hook); > lttv_hooks_add(event_hook, process_event, NULL, LTTV_PRIO_DEFAULT); > > -// result = lttv_iattribute_find_by_path(attributes, "hooks/trace/before", > -// LTTV_POINTER, &value); > -// g_assert(result); > -// before_trace = *(value.v_pointer); > -// g_assert(before_trace); > -// lttv_hooks_add(before_trace, write_trace_header, NULL, > LTTV_PRIO_DEFAULT); > -// > result = lttv_iattribute_find_by_path(attributes, "hooks/traceset/before", > LTTV_POINTER, &value); > g_assert(result); > @@ -2077,7 +2030,7 @@ static void destroy() > g_string_free(a_string, TRUE); > > lttv_hooks_remove_data(event_hook, write_event_content, NULL); > -// lttv_hooks_remove_data(before_trace, write_trace_header, NULL); > + > lttv_hooks_remove_data(before_traceset, write_traceset_header, NULL); > lttv_hooks_remove_data(after_traceset, write_traceset_footer, NULL); > } > diff --git a/lttv/modules/text/precomputeState.c > b/lttv/modules/text/precomputeState.c > index a6e015b..3314d16 100644 > --- a/lttv/modules/text/precomputeState.c > +++ b/lttv/modules/text/precomputeState.c > @@ -40,10 +40,6 @@ > #include <stdio.h> > > static gboolean > - a_field_names, > - a_state, > - a_cpu_stats, > - a_process_stats, > a_raw; > > static char > @@ -91,7 +87,6 @@ static gboolean write_traceset_header(void *hook_data, void > *call_data) > > static gboolean write_traceset_footer(void *hook_data, void *call_data) > { > - LttvTracesetContext *tc = (LttvTracesetContext *)call_data; > GQuark q; > const gchar *string; > > @@ -149,7 +144,6 @@ static gboolean write_trace_header(void *hook_data, void > *call_data) > > static gboolean write_trace_footer(void *hook_data, void *call_data) > { > - LttvTraceContext *tc = (LttvTraceContext *)call_data; > > if(a_raw) { > > @@ -165,25 +159,19 @@ static int for_each_event(void *hook_data, void > *call_data) > { > guint *event_count = (guint*)hook_data; > > - LttvIAttribute *attributes = LTTV_IATTRIBUTE(lttv_global_attributes()); > - > LttvTracefileContext *tfc = (LttvTracefileContext *)call_data; > > LttvTracefileState *tfs = (LttvTracefileState *)call_data; > > LttEvent *e; > > - LttvAttributeValue value_filter; > - > /* Only save at LTTV_STATE_SAVE_INTERVAL */ > if(likely((*event_count)++ < LTTV_STATE_SAVE_INTERVAL)) > return FALSE; > else > *event_count = 0; > > - guint cpu = tfs->cpu; > LttvTraceState *ts = (LttvTraceState*)tfc->t_context; > - LttvProcessState *process = ts->running_process[cpu]; > > e = ltt_tracefile_get_event(tfc->tf); > > diff --git a/lttv/modules/text/sstack.h b/lttv/modules/text/sstack.h > index 184a11d..8689686 100644 > --- a/lttv/modules/text/sstack.h > +++ b/lttv/modules/text/sstack.h > @@ -68,6 +68,7 @@ struct sstack_item *sstack_item_new(void); > struct sstack_item *sstack_item_new_push(unsigned char finished); > struct sstack_item *sstack_item_new_pop(void); > struct sstack_item *sstack_item_new_event(void); > +void sstack_force_flush(struct sstack *stack); > > extern void print_stack(struct sstack *stack); > > diff --git a/lttv/modules/text/sync_chain_batch.c > b/lttv/modules/text/sync_chain_batch.c > index 31fde5e..38e76cd 100644 > --- a/lttv/modules/text/sync_chain_batch.c > +++ b/lttv/modules/text/sync_chain_batch.c > @@ -356,7 +356,7 @@ void teardownSyncChain(LttvTracesetContext* const > traceSetContext) > > if (fclose(syncState->graphsStream) != 0) > { > - g_error(strerror(errno)); > + g_error("%s",strerror(errno)); Missing space. Thanks, Mathieu > } > } > > -- > 1.7.2.3 > > > _______________________________________________ > ltt-dev mailing list > [email protected] > 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 [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
