stdio: provide _unlocked variants of fflush, fputc, fputs, fread, fwrite

[kevans]

Summary

fflush_unlocked is currently desired in ports by sysutils/metalog, and redefined as the locked fflush.

fputc_unlocked, fputs_unlocked, fread_unlocked, and fwrite_unlocked are currently desired in ports by devel/elfutils, and redefined as the locked fputs, fread, and fwrite respectively.

[kib]

include/stdio.h

524 ↗

(On Diff #67211)

I think that fflush/fputs/fread/fwrite redefinitions are not needed. They expand directly into the functions calls so what is the point of providing them ?

I am not aware of any requirements that libc stdio functions were also available as macros. It is reverse, if any std symbol is defined as macro, then it should be also provided as function.

lib/libc/stdio/fflush.c

105 ↗

(On Diff #67211)

I am curious, why strong ?

[kevans]

include/stdio.h

524 ↗

(On Diff #67211)

This was explained up in the message -- the intention is to to ease transition for the couple of ports that attempt to #define them today, so we can just detect their presence in preproc.

lib/libc/stdio/fflush.c

105 ↗

(On Diff #67211)

Admittedly, no thought was put into this decision -- I'll change it to a weak ref, since there's not really a strong case for it to be strong.

[kib]

include/stdio.h

524 ↗

(On Diff #67211)

I must admit that the pre-last sentence only confused me.

I do not insist but it is strange.

[kevans]

include/stdio.h

524 ↗

(On Diff #67211)

Re-reading it, the wording is definitely bad. I was mainly attempting to capture that it's provided even for these that rely on libc for the implementation (i.e. no inline implementation like fputc_unlocked/__sputc) for the sake of detecting in preprocessor stage rather than requiring autoconf goop.

I'll re-assess the ports patches and likely go with __FreeBSD_version checks instead, dropping all of these except for fputc_unlocked. There's only two ports that care, so it's not a large amount of work to find a better solution.

[rpokala]

lib/libc/stdio/fflush.3

78 ↗

(On Diff #67211)

How does using the unlocked version prevent races? If multiple threads use fflush_unlocked, then doesn't that create a race where their updates might be interleaved?

(Ditto for the other manpages.)

[kevans]

lib/libc/stdio/fflush.3

78 ↗

(On Diff #67211)

It's unclear to me what race these refer to (I copied the wording from the existing _unlocked manpage(s) to be consistent), but I imagine it's really wanting to talk about some potential deadlock scenarios.

[jilles]

What is the point of fputc_unlocked() given that putc_unlocked() already exists?

lib/libc/stdio/fflush.3

78 ↗

(On Diff #67211)

I think the idea is to lock the stream once for multiple operations, so they are not interleaved with operations from other threads. However, this does not strictly require _unlocked versions since stream locks are recursive anyway.

[kevans]

In D23336#511952, @jilles wrote:

What is the point of fputc_unlocked() given that putc_unlocked() already exists?

This one actually should've just had the inline macro in the header, most likely, so I'll drop the libc part. Some software (e.g. elfutils) wants to use the fputc_unlocked spelling, so this provides compatibility name rather than ad-hoc macros in ports.

[rpokala]

lib/libc/stdio/fflush.3

78 ↗

(On Diff #67211)

Ahhh! That makes sense, of course.

But in that case, these functions are actually the _locked variants, in that they expect the lock to already be held! <sigh> Naming things is hard. 🙄

For that matter, should they assert that the lock is already held?

[kevans]

Re-roll:

• use weak instead of strong references; there was no justification for strong
• Drop fputc_unlocked from libc, we can provide it entirely inline as we do putc_unlocked and there is no real need for function version at this time, as far as I can tell
• Dropped the macros that I had included for ease of transition... one of the ports will include <stdio.h> before it defines these so it'll just continue using the wrong ones for now, and I've opened a patch for the other one to do similar and protect fputc_unlocked

Also corrected summary to reflect that I had overlooked use of fputc_unlocked in devel/elfutils as well.

[kevans]

Drop overlooked fputs_unlocked macro