Re: [PATCH] trace-cmd: Check all strdup() return values

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2 Jun 2023 17:52:27 +0200
Markus Elfring <Markus.Elfring@web.de> wrote:

> …
> > +++ b/lib/trace-cmd/trace-input.c  
> …
> > @@ -5967,9 +5974,15 @@ tracecmd_buffer_instance_handle(struct tracecmd_input *handle, int indx)
> >  	new_handle->parent = handle;
> >  	new_handle->cpustats = NULL;
> >  	new_handle->hooks = NULL;
> > -	if (handle->uname)
> > +	if (handle->uname) {
> >  		/* Ignore if fails to malloc, no biggy */
> >  		new_handle->uname = strdup(handle->uname);
> > +		if (new_handle->uname == NULL) {
> > +			free(new_handle->trace_clock);
> > +			free(new_handle);
> > +			return NULL;
> > +		}
> > +	}
> >  	tracecmd_ref(handle);
> >
> >  	new_handle->fd = dup(handle->fd);  
> 
> Did the shown comment line become outdated with this change approach?

No, the comment is correct. Thanks for pointing that out. This is what
I get when I use scripts for changes.

> 
> 
> …
> > +++ b/tracecmd/trace-record.c  
> …
> > @@ -371,8 +373,11 @@ struct buffer_instance
> > *allocate_instance(const char *name) instance = calloc(1,
> > sizeof(*instance)); if (!instance)
> >  		return NULL;
> > -	if (name)
> > +	if (name) {
> >  		instance->name = strdup(name);
> > +		if (instance->name == NULL)
> > +			goto error;
> > +	}
> >  	if (tracefs_instance_exists(name)) {
> >  		instance->tracefs = tracefs_instance_create(name);
> >  		if (!instance->tracefs)  
> 
> I hope that more appropriate labels can be applied.
> 
> See also:
> * Improve exception handling in allocate_instance()
>   https://bugzilla.kernel.org/show_bug.cgi?id=217128
> 
> *
> https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29
> 
> * Completion of error handling
>   https://bugzilla.kernel.org/show_bug.cgi?id=217126

For the application, this is good enough.

-- Steve