RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c (original) (raw)
Thomas Stüfe thomas.stuefe at gmail.com
Tue Mar 27 16:24:32 UTC 2018
- Previous message (by thread): RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c
- Next message (by thread): RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi Magnus,
just a cursory look, will look in greater detail tomorrow.
This is good, thanks for doing this.
As I wrote offlist, please remove the painfully wrong AIX "workarounds". I do not know why we did this - the original code is quite old - my assumption is that dlsym(RTLD_NEXT) was not working as expected on older AIX versions. Well, it should work now according to IBMs manpages. Will test later.
__thread is not Posix. I would prefer pthread_get/set_specific instead, which is more portable.
Thanks!
Thomas
On Tue, Mar 27, 2018 at 11:42 AM, Magnus Ihse Bursie < magnus.ihse.bursie at oracle.com> wrote:
When I was about to update jsig.c, I noticed that the four copies for aix, linux, macosx and solaris were basically the same, with only small differences. Some of the differences were not even well motivated, but were likely the result of this code duplication causing the code to diverge.
A better solution is to unify them all into a single unix version, with the few platform-specific changes handled on a per-platform basis. I've made the following notable changes: * I have removed the version check for Solaris. All other platforms seem to do fine without it, and in general, we don't mistrust other JDK libraries. An alternative is to add this version check to all other platforms instead. If you think this is the correct course of action, let me know and I'll fix it. * Solaris used to have a dynamically allocated sact, instead of a statically allocated array as all other platforms have. It's not likely to be large, and the size is known at compile time, so there seems to be no good reason for this. * Linux and macosx used a uint32t/uint64t instead of sigsett for jvmsigs, as aix and solaris do. This is a less robust solution, and the added checks show that it has failed in the past. Now all platforms use sigsett/sigismember(). Also worth noting: * Solaris is not using pthreads, but it's own thread library, which accounts for most of the #ifdef SOLARIS. * In general, if an implementation was needed on one platform, but has no effect or is harmless on others, I've kept it on all platforms instead of sprinkling the code with #ifdefs. To facilitate code review, here is a specially crafted webrev that shows the differences compared to each of the individual, original per-OS versions of jsig.c: http://cr.openjdk.java.net/~ihse/JDK-8200298-unify-libjsig/webrev.01 Bug: https://bugs.openjdk.java.net/browse/JDK-8200298 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8200298-unify-libjsig/ webrev.03 /Magnus
- Previous message (by thread): RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c
- Next message (by thread): RFR (M): JDK-8200298 Unify all unix versions of libjsig/jsig.c
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]