Re: [PATCH 2/9] tools lib traceevent: Copy trace_clock and free it

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Mar 24, 2015 at 09:57:50AM -0400, Steven Rostedt escreveu:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> The pevent->trace_clock should not be a direct pointer to what
> was given. It should be copied and freed.
> 
> Note, valgrind pointed this out when a caller passed in a pointer
> that needed to be freed and it never was. Ideally, pevent should
> copy it (which this change does), and free the copy. It's up
> to the caller to free the clock string passed in.

> +++ b/tools/lib/traceevent/event-parse.c
> @@ -321,9 +321,14 @@ int pevent_register_comm(struct pevent *pevent, const char *comm, int pid)
>  	return 0;
>  }
>  
> -void pevent_register_trace_clock(struct pevent *pevent, char *trace_clock)
> +int pevent_register_trace_clock(struct pevent *pevent, const char *trace_clock)
>  {
> -	pevent->trace_clock = trace_clock;
> +	pevent->trace_clock = strdup(trace_clock);
> +	if (!pevent->trace_clock) {
> +		errno = ENOMEM;
> +		return -1;

Humm, strdup actually sets errno already, from its man page:

-------------------

RETURN VALUE
       On success, the strdup() function returns a pointer to the
duplicated string.  It returns NULL if insufficient memory was
available, with errno set to indicate the cause of the error.

-------------------


Applying anyway, as this doesn't introduces a problem, right?

- ARnaldo