On Sat, May 24, 2025, at 22:28, Niels Dossche wrote:
> On 24/05/2025 21:24, Rob Landers wrote:
> >
> >
> > On Sat, May 24, 2025, at 19:42, Niels Dossche wrote:
> >> Hi
> >>
> >> In my opinion, the return type should not be nullable.
> >> Returning NULL when the platform (or PHP on that platform) doesn't support
> >> getting this information is an anti-pattern.
> >> Instead, availability of the necessary functionality should be checked at configure
> >> time and the function should be made conditionally available.
> >> That way, the return type can just be "int".
> >>
> >> Kind regards
> >> Niels
> >>
> >
> > I’m curious why you say this is an “anti pattern”? I do agree that it should return
> > a number or throw.
>
> If you make the function unconditionally available, yet specifying the return type as ?int,
> then you give the false impression that it _can_ work even if your platform doesn't support it.
>
> Also: IMO new APIs should fail hard with an exception if they can't do their main task,
> that's our error "channel".
>
> > There are various error conditions it should throw (at least on Linux) so having it throw
> > an exception when there isn’t a way to count any processors makes more sense than returning null.
>
> Throwing is indeed preferable.
> What are the error conditions on Linux?
>
> >
> > It could also be someone patching libc to get around per-core licensing too. I’ve seen
> > the latter more often than the former.
> I don't follow.
> In any case, if you run a monkey patched libc and you're breaking the consumer
> expectations of said libc, then you deserve getting it blown up in your face imo.
>
> Kind regards
> Niels
>
I think we both agree it should blow up.
Error conditions (for Linux) using the current method in the patch are either -1 or 0. -1 is just a
possible return for that function which could indicate that the kernel doesn’t have support for
that sysconf value or there are infinite cores — you’d have to check errno.
It might also report 0, which technically isn’t an error (hotplugging cpus for instance). It could
also just be a bug. This is a fun one: https://github.com/lxc/lxcfs/issues/469
Also, I wouldn’t be surprised to see a container with very small amounts of cpu allocated getting
rounded to zero. But in any case, zero should probably be treated as an error IMHO.
— Rob