Re: [PATCH 2/6] lsmem: add support to display dynamic (de)configuration of memory

Sumanth Korikkar <[email protected]> Thu, 16 Oct 2025 14:04:50 +0200
Newsgroups org.kernel.vger.util-linux
Message-ID <[email protected]>
On Thu, Oct 16, 2025 at 01:11:09PM +0200, Karel Zak wrote:
> On Thu, Oct 16, 2025 at 12:16:49PM +0200, Sumanth Korikkar wrote:
> > +static bool skip_memconfig_column(struct lsmem *lsmem, int i)
> > +{
> > +	struct coldesc *ci = get_column_desc(i);
> > +
> > +	if (!strcmp(ci->name, "MEMMAP-ON-MEMORY") || !strcmp(ci->name, "CONFIGURED")) {
> 
> Using column name strings seems fragile. It's better to use column ID
> macros to ensure the compiler verifies them.
> 
>     int id = get_column_id(num);
> 
>     if (id == COL_MEMMAP || id == COL_CONFIG)  

Sure.

> > +		if (ul_path_access(lsmem->sysmemconfig, F_OK, "memory0") == 0)
> > +			return false;
> 
> Do we need this access call? It seems like a duplicate of
> lsmem->have_memconfig, which you initialized in read_basic_info()
> using the same access() call.

Right. I will use lsmem->have_memconfig.

> > +static void read_memconfig(struct lsmem *lsmem)
> > +{
> > +	char dir[PATH_MAX];
> > +
> > +	if (ul_path_access(lsmem->sysmemconfig, F_OK, "memory0") != 0) {
> 
> The same applies if (lsmem->have_memconfig) is sufficient.

Noted. Thanks