On Thu, Nov 19, 2020 at 09:38:20AM +0100, Karel Zak wrote:
> On Fri, Nov 13, 2020 at 11:12:25PM -0500, Masayoshi Mizuma wrote:
> > cpu_column_name_to_id(const char *name, size_t namesz)
> > {
> > size_t i;
> > + int is_cluster = lscpu_is_cluster_arm(NULL);
> >
> > for (i = 0; i < ARRAY_SIZE(coldescs_cpu); i++) {
> > const char *cn = coldescs_cpu[i].name;
> >
> > - if (!strncasecmp(name, cn, namesz) && !*(cn + namesz))
> > + if (!strncasecmp(name, cn, namesz) && !*(cn + namesz)) {
> > + if ((!strncasecmp(cn, "cluster", namesz)) && (!is_cluster)) {
> > + warnx(_("%s doesn't work on this machine. Use socket."), name);
> > + return -1;
> > + } else if ((!strncasecmp(cn, "socket", namesz)) && (is_cluster)) {
> > + warnx(_("%s doesn't work on this machine. Use cluster."), name);
> > + return -1;
> > + }
>
> This is very unusual for your utils and it makes scripts with lscpu
> non-portable. It would be better to remove this change.
>
> We usually follow the columns as specified by user and if we can't
> fill any data then we return nothing (or "-"). For example you can use
> "-o DRAWER" on system where this stuff is not supported.
Thanks, got it. I'll remove this change.
>
> > return i;
> > + }
> > }
> > warnx(_("unknown column: %s"), name);
> > return -1;
> > @@ -337,6 +348,7 @@ static char *get_cell_data(
> > fill_id(cxt, cpu, core, buf, bufsz);
> > break;
> > case COL_CPU_SOCKET:
> > + case COL_CPU_CLUSTER:
> > fill_id(cxt, cpu, socket, buf, bufsz);
> > break;
>
> What about:
>
> case COL_CPU_SOCKET:
> fill_id(cxt, cpu, socket, buf, bufsz);
> break;
> case COL_CPU_CLUSTER:
> if (cxt->is_cluster)
> fill_id(cxt, cpu, socket, buf, bufsz);
> break;
>
> It means "SOCKET" works everywhere, "CLUSTER" returns data only on
> ARMs with cluster(s).
Great idea! I'll fix it as that.
Thanks!
Masa
