Re: [PATCH v3] perf: Add layout support for --symfs option

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Mar 09, 2026 at 08:47:59AM +0000, duchangbin wrote:
> On Thu, Mar 05, 2026 at 05:02:17PM +0800, duchangbin wrote:
> > On Wed, Mar 04, 2026 at 10:48:00PM -0800, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > > @@ -2491,16 +2492,36 @@ int symbol__config_symfs(const struct option *opt __maybe_unused,
> > > >  			 const char *dir, int unset __maybe_unused)
> > > >  {
> > > >  	char *bf = NULL;
> > > > +	char *layout_str;
> > > >  	int ret;
> > > >  
> > > > -	symbol_conf.symfs = strdup(dir);
> > > > -	if (symbol_conf.symfs == NULL)
> > > > -		return -ENOMEM;
> > > > +	layout_str = strrchr(dir, ',');
> > > > +	if (layout_str &&
> > > > +	    (!strcmp(layout_str + 1, "flat") || !strcmp(layout_str + 1, "hierarchy"))) {
> > > 
> > > I think it's better to fail with a message if unknown layout is given.
> > >
> > Agreed. The current issue is that if we use ',' as the delimiter (to maintain a
> > consistent style with other options), and the path itself contains a comma, it
> > would result in the following ambiguity:
> >  - /path,flat → symfs=/path, layout=flat
> >  - /path,hierarchy → symfs=/path, layout=hierarchy
> >  - /some,path/ → symfs=/some, layout=path/
> > 
> > Do you have better suggestions? Using a different delimiter (e.g., : or ;)?
> > 
> Finally, I've decided to go with the simplest implementation. The result is: if
> the path contains a comma, the layout must be specified, otherwise an error will
> be raised (the part after the comma would be incorrectly recognized as the
> layout option).
> 
> - /path → symfs=/path, layout=hierarchy
> - /path,flat → symfs=/path, layout=flat
> - /some,path/ → error
> - /some,path/,flat → symfs=/some,path/, layout=flat

LGTM.

Thanks,
Namhyung