I had tried to create the temporary directory name without dependency on /dev/random (and thus suggested to use "ps | sha256", or in my temporary version "ps lauxww | grep sha256").

As written in the comment of the original submission, a version based on "df /tmp | while ..." can be used instead of the here-doc.
But after implementing both, I did consider the here-doc based version to be both easier to understand and more efficient and thus only proposed that version.

Remove dependency on mktemp, too, since it also resides in /usr/bin, as noted by rgrimes.

In D27562#620824, @rlibby wrote:

@se Now that vendor workflows are open for git, can we get upstream's fix in? Or do you prefer I just do a direct commit of upstream's fix as discussed above?

I have updated the math/gh-bc port to version 3.2.4, which includes a slightly modified version of this change.
The author wanted to avoid problems with other compilers than gcc and clang, which might not support the __has_attribute(()) mechanism, and therefore has added further conditions to your patch.

In D27568#616080, @arichardson wrote:

FYI, I have commit access and could have committed this myself.

Thank you for the report and sorry for the breakage - is there a mail list that reports new test failures?

Since neither the statically allocated 1024 char buffer nor the version with the constructor has made the reviewers happy, I'm going back to the 2nd proposed version, which does only perform system calls when this function is actually called and allocates a minimally sized buffer only when necessary to hold the sysctl result.

In D27236#608681, @cem wrote:

In D27236#608678, @imp wrote:

In D27236#608561, @hselasky wrote:

If you ktrace a regular application starting up you already see a lot of system calls, so I think adding one more is fine.

Except there's many efforts to reduce this. At present, this function is called by a tiny number of binaries, so I'd rather have any overhead be limited to those programs that use it.

+1

We don't want to saddle every program that links libutil with a constructor like this. Instead, construct on demand. This could be relatively trivial: drop the ctor attribute, and instead:

In D27236#608678, @imp wrote:

In D27236#608561, @hselasky wrote:

If you ktrace a regular application starting up you already see a lot of system calls, so I think adding one more is fine.

Except there's many efforts to reduce this. At present, this function is called by a tiny number of binaries, so I'd rather have any overhead be limited to those programs that use it.

In D27236#608562, @hselasky wrote:

MIght be we also need a destructor to free the memory, if any, to make valgrind happy.

In D27236#608551, @hselasky wrote:

Looks good to me. I hope it does not affect the performance of any apps, that are launched frequently.

Hans Petter Selasky suggested the use of a constructor, something that I had not thought about at all ...

Updated diff with an alternate implementation that does not include a static buffer of length MAXPATHLEN.
(The actual memory requirements of that alternative implementation are larger than those of the version with the static buffer, though.)

In D27236#608246, @imp wrote:

In D27236#608193, @cem wrote:

The user.localbase sysctl seems like a poor design, and doesn't exist at the moment. That needs to be resolved before this change is committed.

Add another invocation of getlocalbase() instead of getenv("LOCALBASE") in usr.sbin/pkg

In D27022#607192, @bdrewery wrote:

The problem I see is that not everything will use this or the sysctl. Ports, Poudriere, nearly everything, are doing getenv("LOCALBASE"). Other places have things like LOCALBASE?= /usr/local. So then I wonder what the usefulness of it being configured in the kernel in the first place is as a user setting it won't achieve much.

I might argue it makes more sense to make it call this getlocalbase(3) when getenv("LOCALBASE") is done but that's crazy right? LOCALBASE typically should be set once on a system down in something like /etc/login.conf and otherwise exported in build scripts.

I certainly understand the want to unify all of these LOCALBASE?= /usr/local and _PATH_LOCALBASE things but the interface really needs to be prioritizing the environment where its primary consumer is looking.

I'm still in doubt about the usefulness of LOCALBASE being specified at run-time if it is not applied to all paths in the system.
And for all other cases (not changed at run-time) a compiled in _PATH_LOCALBASE works as well, while scripts can use the sysctl command.