This is a bunch of changes from Apple's Libc.
I am specifically excluding xlocale() and collation changes
and I am focusing on UNIX03 and compliance issues.
Details
This is an initial patch, I only know it compiles but it
may still have style issues and even mistakes.
Some of this, in particular the freopen changes will require
more testing. I will be running the gnulib tests on it.
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2445 Build 2462: arc lint + arc unit
Event Timeline
Some parts are good, some parts need changes, some parts may be wrong. See per-file comments.
This should be multiple commits.
Note that I have only looked at the code. I have not tested it in any way.
lib/libc/stdio/fgetln.3 | ||
---|---|---|
102 | Perhaps the INT_MAX part should instead be in a BUGS section. It is unlikely to be fixed. Applications can use getline() instead. | |
lib/libc/stdio/fputs.c | ||
67 | This is wrong if the length exceeds INT_MAX. POSIX.1-2008tc1 suggests capping at INT_MAX if the length is supposed to be returned (this text is new in tc1). Note that the standard text simply requires a non-negative integer if successful, so this is likely another bug in the testsuite :( | |
lib/libc/stdio/freopen.c | ||
100 ↗ | (On Diff #763) | This errno change can probably committed separately, as it seems obviously correct to me. |
135 ↗ | (On Diff #763) | Do we want to break compatibility in this way? An freopen() of stdin/stdout/stderr is common, and dangerous bugs may show up if those streams lose their common fd numbers. |
155 ↗ | (On Diff #763) | We may want this (even though it is dead code with the above change), but only for fd >= 3. |
lib/libc/stdio/wbuf.c | ||
64 ↗ | (On Diff #763) | The prepwrite macro is used six more times; should those be adjusted as well? |
Thank you.
Yes,the changes are meant be committed individually. This is all just work-in-progress.
I am undecided still on freopen(), I think it should probably be discussed in the lists.
I assume the rest is OK ... so I will do some more checking and I will commit them slowly.
FWIW, we pass the gnulib tests before and after the changes (with the exception of a
coredump in test-striconv, which seems unrelated).
lib/libc/stdio/fgetln.3 | ||
---|---|---|
102 | I will consider it for a different commit that touches only documentation, Thank you. | |
lib/libc/stdio/fputs.c | ||
67 | Good ... I do prefer to keep things simple :). | |
lib/libc/stdio/freopen.c | ||
100 ↗ | (On Diff #763) | Committed revision 268926. |
135 ↗ | (On Diff #763) | It is a good question. I guess AIX and whatever "big iron" UNIXes are left already do this. One possibility would be to bring it to current but no MFC. On the other hand we are not forced to comply with UNIX 03. |
Some more replies to your comments.
lib/libc/stdio/fputs.c | ||
---|---|---|
67 | Actually. I have a new patch with this: | |
lib/libc/stdio/wbuf.c | ||
64 ↗ | (On Diff #763) | Good catch: Apple also adds the adjustment on vfprintf.c. I added it to vfwprintf.c too. The two remaining checks according to preceeding comments in the code "may not be necessary". |
- Updating D442: libc/stdio changes from Apple #
These are the remaining *controversial* changes:
- The fputs() change should be committed but not MFC'd.
- The freopen() changes will have to be rethought (help welcome).
- The fgetln() changes will not be committed: are here only for reference.
- Updating D442: libc/stdio changes from Apple #
- Enter a brief description of the changes included in this update.
- The first line is used as subject, next lines as comment. #
Rescue what seems to be good from the freopen patch:
rewritten by ache@, pretty much in line with jilles@ comments.
Add a small enhancemente to fpurge.
Try to add ache as a reviewer:
Reviewers:
ache
- Updating D442: libc/stdio changes from Apple #
- Enter a brief description of the changes included in this update.
- The first line is used as subject, next lines as comment. #
Nevermind about the fpurge change: it is a local change corresponding
to r236288.
- Updating D442: libc/stdio changes from Apple #
- Enter a brief description of the changes included in this update.
- The first line is used as subject, next lines as comment. #
Cleanup: drop changes that have been committed or directly rejected.
These are the remaining changes.
I now think neither of them should be committed.
In the case of fputs() returning 0 upon failure seems more intuitive
than returning the length of the string. While Apple's behavior is
also standards compliant it can lead to compatibility issues.
I am considering to apply this change to FreeBSD 11 as-is:
- The fgetln change is a consequence of a limitation in our implementation: not sure if it classifies as a bug but there's nothing we can do at this time.
- The fputs change is something the standard lets us do and is convenient for compatibility with Apple. A small change would be returning uio.uio_resid since that is an int already.
Committed as r295631 (getln), r295632 (fputs).
No plans to MFC.
No idea why phabricator didn't catch it.