[ltt-dev] [LTTV PATCH] Fix remaining warnings

Mathieu Desnoyers compudj at krystal.dyndns.org
Tue Jan 25 19:59:04 EST 2011


* Alexandre Montplaisir (alexandre.montplaisir at polymtl.ca) wrote:
> Add and check the return values of fscanf, fread and asprintf functions.
> 
> That should be it! GCC 4.5 with -Wall doesn't report anything else.
> 
> Signed-off-by: Alexandre Montplaisir <alexandre.montplaisir at polymtl.ca>
> ---
>  lttv/lttv/attribute.c           |    8 ++--
>  lttv/lttv/state.c               |   72 ++++++++++++++++++++++++---------------
>  lttv/modules/text/depanalysis.c |   23 +++++++++----
>  3 files changed, 64 insertions(+), 39 deletions(-)
> 
> diff --git a/lttv/lttv/attribute.c b/lttv/lttv/attribute.c
> index 243fe02..3b09468 100644
> --- a/lttv/lttv/attribute.c
> +++ b/lttv/lttv/attribute.c
> @@ -487,13 +487,13 @@ lttv_attribute_read_xml(LttvAttribute *self, FILE *fp)
>  
>  	LttvAttribute *subtree;
>  
> -	fscanf(fp,"<ATTRS>");
> +	res = fscanf(fp, "<ATTRS>");

Is it me or you forgot to check res here ?

>  	while(1) {
>  		res = fscanf(fp, "<ATTR NAME=\"%256[^\"]\" TYPE=%10[^ >]", buffer, type);
>  		g_assert(res == 2);
>  		name = g_quark_from_string(buffer);
>  		if(strcmp(type, "ATTRS") == 0) {
> -			fscanf(fp, ">");
> +			res = fscanf(fp, ">");

and here ?

>  			subtree = lttv_attribute_find_subdir(self, name);
>  			lttv_attribute_read_xml(subtree, fp);
>  		}
> @@ -553,11 +553,11 @@ lttv_attribute_read_xml(LttvAttribute *self, FILE *fp)
>  		}
>  		else if(strcmp(type, "NONE") == 0) {
>  			value = lttv_attribute_add(self, name, LTTV_NONE);
> -			fscanf(fp, "/>");
> +			res = fscanf(fp, "/>");

here,

>  		}
>  		else g_error("Unknown type to read");
>  	}
> -	fscanf(fp,"</ATTRS>");
> +	res = fscanf(fp, "</ATTRS>");

and here ?

The rest looks good though.

Mathieu

>  }
>  
>  static LttvAttribute *
> diff --git a/lttv/lttv/state.c b/lttv/lttv/state.c
> index bd3f64d..3d3b0a6 100644
> --- a/lttv/lttv/state.c
> +++ b/lttv/lttv/state.c
> @@ -578,6 +578,7 @@ static void state_load_saved_states(LttvTraceState *tcs)
>  	gint hdr;
>  	gchar buf[MAX_STRING_LEN];
>  	guint len;
> +	size_t res;
>  
>  	trace_path = g_quark_to_string(ltt_trace_name(tcs->parent.t));
>  	strncpy(path, trace_path, PATH_MAX-1);
> @@ -600,7 +601,8 @@ static void state_load_saved_states(LttvTraceState *tcs)
>  		g_ptr_array_set_size(quarktable, q+1);
>  		i=0;
>  		while(1) {
> -			fread(&buf[i], sizeof(gchar), 1, fp);
> +			res = fread(&buf[i], sizeof(gchar), 1, fp);
> +			g_assert(res == 1);
>  			if(buf[i] == '\0' || feof(fp)) break;
>  			i++;
>  		}
> @@ -1053,20 +1055,21 @@ static void read_process_state_raw(LttvTraceState *self, FILE *fp,
>  	LttvProcessState *process, *parent_process;
>  	LttvProcessState tmp;
>  	GQuark tmpq;
> +	size_t res;
>  
>  	guint64 *address;
>  
> -	/* TODO : check return value */
> -	fread(&tmp.type, sizeof(tmp.type), 1, fp);
> -	fread(&tmp.name, sizeof(tmp.name), 1, fp);
> -	fread(&tmp.brand, sizeof(tmp.brand), 1, fp);
> -	fread(&tmp.pid, sizeof(tmp.pid), 1, fp);
> -	fread(&tmp.free_events, sizeof(tmp.free_events), 1, fp);
> -	fread(&tmp.tgid, sizeof(tmp.tgid), 1, fp);
> -	fread(&tmp.ppid, sizeof(tmp.ppid), 1, fp);
> -	fread(&tmp.cpu, sizeof(tmp.cpu), 1, fp);
> -	fread(&tmp.creation_time, sizeof(tmp.creation_time), 1, fp);
> -	fread(&tmp.insertion_time, sizeof(tmp.insertion_time), 1, fp);
> +	res = fread(&tmp.type, sizeof(tmp.type), 1, fp);
> +	res += fread(&tmp.name, sizeof(tmp.name), 1, fp);
> +	res += fread(&tmp.brand, sizeof(tmp.brand), 1, fp);
> +	res += fread(&tmp.pid, sizeof(tmp.pid), 1, fp);
> +	res += fread(&tmp.free_events, sizeof(tmp.free_events), 1, fp);
> +	res += fread(&tmp.tgid, sizeof(tmp.tgid), 1, fp);
> +	res += fread(&tmp.ppid, sizeof(tmp.ppid), 1, fp);
> +	res += fread(&tmp.cpu, sizeof(tmp.cpu), 1, fp);
> +	res += fread(&tmp.creation_time, sizeof(tmp.creation_time), 1, fp);
> +	res += fread(&tmp.insertion_time, sizeof(tmp.insertion_time), 1, fp);
> +	g_assert(res == 10);
>  
>  	if(tmp.pid == 0) {
>  		process = lttv_state_find_process(self, tmp.cpu, tmp.pid);
> @@ -1109,18 +1112,22 @@ static void read_process_state_raw(LttvTraceState *self, FILE *fp,
>  						process->execution_stack->len-1);
>  				process->state = es;
>  
> -				fread(&es->t, sizeof(es->t), 1, fp);
> +				res = fread(&es->t, sizeof(es->t), 1, fp);
> +				g_assert(res == 1);
>  				es->t = g_quark_from_string(
>  						(gchar*)g_ptr_array_index(quarktable, es->t));
> -				fread(&es->n, sizeof(es->n), 1, fp);
> +				res = fread(&es->n, sizeof(es->n), 1, fp);
> +				g_assert(res == 1);
>  				es->n = g_quark_from_string(
>  						(gchar*)g_ptr_array_index(quarktable, es->n));
> -				fread(&es->s, sizeof(es->s), 1, fp);
> +				res = fread(&es->s, sizeof(es->s), 1, fp);
> +				g_assert(res == 1);
>  				es->s = g_quark_from_string(
>  						(gchar*)g_ptr_array_index(quarktable, es->s));
> -				fread(&es->entry, sizeof(es->entry), 1, fp);
> -				fread(&es->change, sizeof(es->change), 1, fp);
> -				fread(&es->cum_cpu_time, sizeof(es->cum_cpu_time), 1, fp);
> +				res = fread(&es->entry, sizeof(es->entry), 1, fp);
> +				res += fread(&es->change, sizeof(es->change), 1, fp);
> +				res += fread(&es->cum_cpu_time, sizeof(es->cum_cpu_time), 1, fp);
> +				g_assert(res == 3);
>  				break;
>  
>  			case HDR_USER_STACK:
> @@ -1128,13 +1135,16 @@ static void read_process_state_raw(LttvTraceState *self, FILE *fp,
>  						process->user_stack->len + 1);
>  				address = &g_array_index(process->user_stack, guint64,
>  						process->user_stack->len-1);
> -				fread(address, sizeof(address), 1, fp);
> +				res = fread(address, sizeof(address), 1, fp);
> +				g_assert(res == 1);
>  				process->current_function = *address;
>  				break;
>  
>  			case HDR_USERTRACE:
> -				fread(&tmpq, sizeof(tmpq), 1, fp);
> -				fread(&process->usertrace->cpu, sizeof(process->usertrace->cpu), 1, fp);
> +				res = fread(&tmpq, sizeof(tmpq), 1, fp);
> +				res += fread(&process->usertrace->cpu,
> +						sizeof(process->usertrace->cpu), 1, fp);
> +				g_assert(res == 2);
>  				break;
>  
>  			default:
> @@ -1160,6 +1170,7 @@ void lttv_state_read_raw(LttvTraceState *self, FILE *fp, GPtrArray *quarktable)
>  	guint nb_cpus;
>  
>  	int hdr;
> +	size_t res;
>  
>  	LttTime t;
>  
> @@ -1171,7 +1182,8 @@ void lttv_state_read_raw(LttvTraceState *self, FILE *fp, GPtrArray *quarktable)
>  
>  	restore_init_state(self);
>  
> -	fread(&t, sizeof(t), 1, fp);
> +	res = fread(&t, sizeof(t), 1, fp);
> +	g_assert(res == 1);
>  
>  	do {
>  		if(feof(fp) || ferror(fp)) goto end_loop;
> @@ -1208,10 +1220,12 @@ end_loop:
>  		int cpu_num;
>  		hdr = fgetc(fp);
>  		g_assert(hdr == HDR_CPU);
> -		fread(&cpu_num, sizeof(cpu_num), 1, fp); /* cpu number */
> +		res = fread(&cpu_num, sizeof(cpu_num), 1, fp); /* cpu number */
> +		g_assert(res == 1);
>  		g_assert(i == cpu_num);
> -		fread(&self->running_process[i]->pid,
> +		res = fread(&self->running_process[i]->pid,
>  				sizeof(self->running_process[i]->pid), 1, fp);
> +		g_assert(res == 1);
>  	}
>  
>  	nb_tracefile = self->parent.tracefiles->len;
> @@ -1226,13 +1240,15 @@ end_loop:
>  		g_tree_remove(pqueue, &tfcs->parent);
>  		hdr = fgetc(fp);
>  		g_assert(hdr == HDR_TRACEFILE);
> -		fread(&tfcs->parent.timestamp, sizeof(tfcs->parent.timestamp), 1, fp);
> +		res = fread(&tfcs->parent.timestamp, sizeof(tfcs->parent.timestamp), 1, fp);
> +		g_assert(res == 1);
>  		/* Note : if timestamp if LTT_TIME_INFINITE, there will be no
>  		 * position following : end of trace */
>  		if(ltt_time_compare(tfcs->parent.timestamp, ltt_time_infinite) != 0) {
> -			fread(&nb_block, sizeof(nb_block), 1, fp);
> -			fread(&offset, sizeof(offset), 1, fp);
> -			fread(&tsc, sizeof(tsc), 1, fp);
> +			res = fread(&nb_block, sizeof(nb_block), 1, fp);
> +			res += fread(&offset, sizeof(offset), 1, fp);
> +			res += fread(&tsc, sizeof(tsc), 1, fp);
> +			g_assert(res == 3);
>  			ltt_event_position_set(ep, tfcs->parent.tf, nb_block, offset, tsc);
>  			gint ret = ltt_tracefile_seek_position(tfcs->parent.tf, ep);
>  			g_assert(ret == 0);
> diff --git a/lttv/modules/text/depanalysis.c b/lttv/modules/text/depanalysis.c
> index 2019038..e9359df 100644
> --- a/lttv/modules/text/depanalysis.c
> +++ b/lttv/modules/text/depanalysis.c
> @@ -753,15 +753,18 @@ static inline void print_pid(int pid)
>  static void modify_path_with_private(GArray *path, struct process_state *pstate)
>  {
>  	char *tmps;
> +	int res;
>  
>  	// 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((GQuark)(unsigned long)g_hash_table_lookup(irq_table, &((struct hlev_state_info_interrupted_irq *)pstate->private)->irq)));
> +			res = 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_assert(res > 0);
>  			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((GQuark)(unsigned long)g_hash_table_lookup(softirq_table, &((struct hlev_state_info_interrupted_softirq *)pstate->private)->softirq)));
> +			res = 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_assert(res > 0);
>  			g_array_append_val(path, tmps);
>  			break;
>  		case HLEV_BLOCKED: {
> @@ -777,7 +780,8 @@ 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((GQuark)(unsigned long)g_hash_table_lookup(syscall_table, &hlev_blocked_private->syscall_id)));
> +				res = 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_assert(res > 0);
>  				g_array_append_val(path, tmps);
>  			}
>  
> @@ -787,14 +791,16 @@ static void modify_path_with_private(GArray *path, struct process_state *pstate)
>  			}
>  			else if(((struct hlev_state_info_blocked *)pstate->private)->substate == HLEV_BLOCKED__READ) {
>  				char *str;
> -				asprintf(&str, "%s", g_quark_to_string(((struct hlev_state_info_blocked__read *)((struct hlev_state_info_blocked *)pstate->private)->private)->filename));
> +				res = asprintf(&str, "%s", g_quark_to_string(((struct hlev_state_info_blocked__read *)((struct hlev_state_info_blocked *)pstate->private)->private)->filename));
> +				g_assert(res > 0);
>  				g_array_append_val(path, str);
>  				/* FIXME: this must be freed at some point */
>  				//free(str);
>  			}
>  			else if(((struct hlev_state_info_blocked *)pstate->private)->substate == HLEV_BLOCKED__POLL) {
>  				char *str;
> -				asprintf(&str, "%s", g_quark_to_string(((struct hlev_state_info_blocked__poll *)((struct hlev_state_info_blocked *)pstate->private)->private)->filename));
> +				res = asprintf(&str, "%s", g_quark_to_string(((struct hlev_state_info_blocked__poll *)((struct hlev_state_info_blocked *)pstate->private)->private)->filename));
> +				g_assert(res > 0);
>  				g_array_append_val(path, str);
>  				/* FIXME: this must be freed at some point */
>  				//free(str);
> @@ -1565,6 +1571,7 @@ static int process_event(void *hook_data, void *call_data)
>  	LttvTracefileState *tfs = (LttvTracefileState *)call_data;
>  	LttEvent *e;
>  	struct marker_info *info;
> +	int res;
>  
>  	/* Extract data from event structures and state */
>  	guint cpu = tfs->cpu;
> @@ -1804,7 +1811,8 @@ static int process_event(void *hook_data, void *call_data)
>  		}
>  		else {
>  			char *tmp;
> -			asprintf(&tmp, "Unknown filename, fd %d", fd);
> +			res = asprintf(&tmp, "Unknown filename, fd %d", fd);
> +			g_assert(res > 0);
>  			llev_syscall_read_private->filename = g_quark_from_string(tmp);
>  			free(tmp);
>  		}
> @@ -1850,7 +1858,8 @@ static int process_event(void *hook_data, void *call_data)
>  		}
>  		else {
>  			char *tmp;
> -			asprintf(&tmp, "Unknown filename, fd %d", fd);
> +			res = asprintf(&tmp, "Unknown filename, fd %d", fd);
> +			g_assert(res > 0);
>  			llev_syscall_poll_private->filename = g_quark_from_string(tmp);
>  			free(tmp);
>  		}
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> 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