From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752266AbbCXPOw (ORCPT ); Tue, 24 Mar 2015 11:14:52 -0400 Received: from mail.kernel.org ([198.145.29.136]:51537 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751980AbbCXPOu (ORCPT ); Tue, 24 Mar 2015 11:14:50 -0400 Date: Tue, 24 Mar 2015 12:14:49 -0300 From: Arnaldo Carvalho de Melo To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Jiri Olsa , Namhyung Kim , Andrew Morton Subject: Re: [PATCH 2/9] tools lib traceevent: Copy trace_clock and free it Message-ID: <20150324151449.GC5447@kernel.org> References: <20150324135748.506437888@goodmis.org> <20150324135922.695906738@goodmis.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150324135922.695906738@goodmis.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Mar 24, 2015 at 09:57:50AM -0400, Steven Rostedt escreveu: > From: "Steven Rostedt (Red Hat)" > > 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