Re: chainlink.pl /proc/cpuinfo regexp fails on s390x

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 23, 2022, 19:10 UTC
Message-ID: <CAPig+cT5dxrS2-xUSYfot7k6v0qV0TXh=xSMe7Y-oMGLMhbjVA@mail.gmail.com>
In-Reply-To: <mvm1qpu9g2e.fsf@linux-m68k.org>

On Wed, Nov 23, 2022 at 4:27 AM Andreas Schwab <schwab@linux-m68k.org> wrote:

Show 19 quoted lines

> On Nov 22 2022, Eric Sunshine wrote:
> > On Tue, Nov 22, 2022 at 6:37 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >> On Nov 22 2022, Andreas Hasenack wrote:
> >> > /^processor[\s\d]*:/
> >> > or something else.
> >>
> >> Something else.
> >> $ getconf _NPROCESSORS_ONLN
> >
> > Yes, that works too and was (I'm pretty sure) considered during
> > development. The reason /proc/cpu was chosen over `getconf` was that
> > opening & reading /proc/cpu should be faster since it doesn't involve
> > spawning a process. If we did use `getconf`, we'd have to be careful
> > to degrade gracefully if `getconf` isn't available or if the
> > configuration parameter (i.e. "_NPROCESSORS_ONLN") isn't known on the
> > platform.
>
> getconf is surely more portable than poking in /proc, especially
> /proc/cpuinfo is the antipode of portability.

No doubt, but for the immediate issue, this tightly-focused fix is more appropriate and less likely to lead to unexpected additional problems. I'm not arguing against `getconf`, but saying only that _this_ patch is fine as-is.

It may very well be a good idea to replace the /proc/cpuinfo probe by `getconf` in the future (or remove it altogether as mentioned upstream), but that's outside the scope of this patch and the immediate problem.