Re: [PATCH v4 1/7] trace-cmd: Implemented new lib API: tracecmd_local_events_system()

[Steven Rostedt <rostedt@xxxxxxxxxxx>]

On Fri, 22 Feb 2019 17:51:04 +0200
Slavomir Kaslev <kaslevs@xxxxxxxxxx> wrote:

> > @@ -1189,7 +1161,16 @@ int tracecmd_fill_local_events(const char *tracing_dir, struct tep_handle *peven
> >  		if (strcmp(name, ".") == 0 ||
> >  		    strcmp(name, "..") == 0)
> >  			continue;
> > -
> > +		if (sys_names) {
> > +			i = 0;
> > +			while (sys_names[i]) {
> > +				if (strcmp(name, sys_names[i]) == 0)
> > +					break;
> > +				i++;
> > +			}
> > +			if (sys_names[i] == NULL)
> > +				continue;
> > +		}  
> 
> I think this would be better done in a separate function, say:
> 
> static bool contains(const char *name, const char **names)
> {
> 	while (*names) {
> 		if (strcmp(name, *names++) == 0)
> 			return true;
> 	}
> 	return false;
> }
> 
> and then simply have
> 
> if (sys_names && !contains(name, sys_names))
> 	continue;

Actually, I would argue just to make it a for loop.

	for (i = 0; sys_names[i]; i++) {
		if (strcmp(name, sys_names[i]) == 0)
			goto found;
	}
	continue;
found:

But if you really want the helper function, then have it check
sys_names too.

static bool contains(const char *name, const char * const * names)
{
	if (!names)
		return false;
	for (; *names; names++)
		if (strcmp(name, *names) == 0)
			return true;
	return false;
}

	if (!contains(name, sys_names))

-- Steve

> 
> here.
> 
> >  		sys = append_file(events_dir, name);
> >  		ret = stat(sys, &st);
> >  		if (ret < 0 || !S_ISDIR(st.st_mode)) {
> > @@ -1217,6 +1198,58 @@ int tracecmd_fill_local_events(const char *tracing_dir, struct tep_handle *peven
> >  	return ret;
> >  }