Re: [PATCH v3 2/4] util: Introduce read_file() and read_file_long() function

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 29 Oct 2025 11:43:00 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Oct 29, 2025 at 12:12:48AM +0100, Stefano Brivio wrote:
> > On Wed, 15 Oct 2025 15:46:12 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Wed, Oct 15, 2025 at 11:50:53AM +0800, Yumei Huang wrote:  
> > > > On Wed, Oct 15, 2025 at 7:28 AM David Gibson
> > > > <david@gibson.dropbear.id.au> wrote:    
> > > > > On Tue, Oct 14, 2025 at 03:38:34PM +0800, Yumei Huang wrote:    
> > > > > > Signed-off-by: Yumei Huang <yuhuang@redhat.com>    
> > > [snip]  
> > > > > > + * @path: Path to the sysctl file
> > > > > > + * @fallback: Default value if file can't be read
> > > > > > + *
> > > > > > + * Return: Parameter value, fallback on failure
> > > > > > +*/
> > > > > > +long read_file_long(const char *path, long fallback)
> > > > > > +{
> > > > > > +        char buf[32];    
> > > > >
> > > > > Rather than just using a semi-arbitrary 32 here, I'd suggest defining
> > > > > a new constant similar to UINT16_STRLEN.  Except that's trickier for a
> > > > > type that doesn't have a known fixed width.  Pity the C library
> > > > > doesn't have constants for these AFAICT.    
> > > > 
> > > > I will just define a UINTMAX_STRLEN with (sizeof("2147483647")).    
> > > 
> > > That's not quite right.
> > >  - It should be INTMAX_STRLEN (signed), UINTMAX would be for the
> > >    unsigned version
> > >  - That assumes intmax_t is 32-bit which is probably not the case (it
> > >    will be 64-bit, maybe even 128-bit on modern systems)
> > >  - For signed cases, it's the minimum (negative) value that gives the
> > >    longest possible string (for 32-bit, "-2147483648")  
> > 
> > By the way, while it doesn't cover intmax_t explicitly, I think this is
> > a pretty good resource as it covers most architectures supported by the
> > Linux kernel (hence, most architectures we support):
> > 
> >   https://wiki.debian.org/ArchitectureSpecificsMemo#Summary  
> 
> Oh, nice, that is a very handy resource.
> 
> > and judging from intmax_t(3type) I'd say that the sizeof(long double)
> > column tells you how big intmax_t is.  
> 
> > Well, at least, that's the page I use to know which architectures I can
> > use to check things when I suspect a type portability bug.
> > 
> > That's because 'long double' should always be the biggest "native" data
> > type, that is, excluding __int128 or vectorised / SIMD types such as
> > __m256i.  
> 
> This part isn't true, alas.  Theoretically speaking there's not
> necessarily any relation between the largest native integer type and
> the largest native float type.

Oops, yes, I misread intmax_t(3type), that's *integer* only (of course,
the name says it). So probably it has to match sizeof(long long)?

> But more importantly, it's not true in practice: according to the
> table sizeof(long double) is 16 for amd64, but sizeof(intmax_t) is 8
> empirically.
> 
> I think sizeof(long long) is more likely to match sizeof(intmax_t),
> but I don't love relying on it.

Right... well, about relying on it, without a change in the C11
standard, can it ever differ? I don't think so. We could have a look at
C17 / C23 and if long long is still the largest integer type, we know
we're fine for quite a few years / pretty much forever.

By the way, just as a reminder (also to self): we don't actually need
this here.

-- 
Stefano