Re: [PATCH v4 04/10] libtracefs: Change tracefs_kprobe_info API

[Steven Rostedt <rostedt@goodmis.org>]

On Thu,  4 Nov 2021 13:10:41 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> +enum tracefs_dynevent_type tracefs_kprobe_info(struct tracefs_dynevent *kprobe,
> +					       char **system, char **event,
> +					       char **prefix, char **addr, char **format)
> +{
> +	char **lv[] = { system, event, prefix, addr, format };
> +	char **rv[] = { &kprobe->system, &kprobe->event, &kprobe->prefix,
> +			&kprobe->address, &kprobe->format };
> +	int i;
> +
> +	if (!kprobe)
> +		return TRACEFS_DYNEVENT_MAX;
> +
> +	for (i = 0; i < ARRAY_SIZE(lv); i++)
> +		*lv[i] = NULL;

Do we really need to initialize them to NULL here?

Not to mention, if one of the parameters is NULL itself, this will SEGFAULT.

> +
> +	for (i = 0; i < ARRAY_SIZE(lv); i++) {

Nice use of ARRAY_SIZE() ;-)

> +		if (lv[i]) {
> +			if (*rv[i]) {
> +				*lv[i] = strdup(*rv[i]);
> +				if (!*lv[i])
> +					goto error;
> +			} else {
> +				*lv[i] = NULL;
>  			}

The above here sets it to NULL if the parameter is non NULL.

We don't need the initial NULL setting loop.

-- Steve


> -			break;
>  		}
> -		ret = parse_kprobe(NULL, &saveptr, &ktype, &system, &probe,
> -				   &kaddr, &kfmt);
>  	}
> -	free(content);
> -	return rtype;
> +
> +	return kprobe->type;
> +
> +error:
> +	for (i--; i >= 0; i--) {
> +		if (lv[i])
> +			free(*lv[i]);
> +	}
> +
> +	return TRACEFS_DYNEVENT_MAX;
>  }