Make crypt and crypt_gensalt use thread-local output buffers.
This change makes crypt and crypt_gensalt as thread-safe as they can be without changing their interfaces. Solaris already made this change, and it’s being discussed by glibc (with suggestion that it should be pushed upstream to the C and POSIX standards committees): https://sourceware.org/ml/libc-alpha/2018-10/msg00437.html
Portable programs should still use the r-variants, though, because this is not a guaranteed feature that is portable to other implementations of these functions. Also it doesn’t help threads to not clobber their corresponding output buffer on a second call from within the same thread.
The way I've implemented this mimicks the implementation from Solaris.
I've also taken into account some problems with -fno-plt code and thread-local storage on PPC64-architectures. See: https://sourceware.org/PR32387
This also fixes #62.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.45%. Comparing base (
353e507) to head (f76672a).
Additional details and impacted files
@@ Coverage Diff @@
## develop #201 +/- ##
===========================================
+ Coverage 90.44% 90.45% +0.01%
===========================================
Files 36 36
Lines 3988 3993 +5
Branches 747 748 +1
===========================================
+ Hits 3607 3612 +5
Misses 242 242
Partials 139 139
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@besser82 I should have time to look at this early next week, but not before that.
I approve of the goal here. It's something I wanted to do myself quite some time ago
If implementations look so risky and/or complicated to you, then I'm puzzled that you think the benefits outweigh the risks and effort and complexity. Maybe just don't do this?
I think we should use
pthread_getspecificto acquire one thread-local pointer for each function, and allocate the bulk of the data withmalloc
This sounds somewhat similar to what I saw Solaris do, https://github.com/besser82/libxcrypt/pull/62#issuecomment-2588580961
Why care about old glibc versions? It's not like people normally build libxcrypt to replace the one shipped by their distribution.
@rfc1036 The entire point of this library is to be a drop-in replacement for old glibc's libcrypt. If it didn't need to be that, it would have a completely different API.
Of course, but reasonably nobody is replacing glibc's libcrypt on existing systems!
Not true, people do it all the time, just look through the outstanding bug reports.