On 09/28/2018 02:55 PM, Volker Simonis wrote: > I agree that the native code should be fixed and I think your patch is > correct. > > Nevertheless I'd vote for changing it to something like: > > old = (tio.c_lflag & ECHO) == 0 ? JNI_FALSE : JNI_TRUE; > > It's slightly more verbose, but it makes it more obvious that a JNI > function returning a jboolean should only ever return JNI_FALSE or > JNI_TRUE. This isn't obvious from your fix - especially not for C > programmers who produce such code.
Where is the evidence that a function returning a jboolean should only ever return JNI_FALSE or JNI_TRUE? It doesn't say so in the JNI spec, only that these values are provided for convenience. They are defined to be 0 and 1, like so: #define JNI_FALSE 0 #define JNI_TRUE 1 which are *exactly* the same values that the C != operation returns. Therefore there is no advantage either in correctness or clarity gained by using ? JNI_FALSE : JNI_TRUE It's not even pedantically correct, merely verbose. > Also, it would be good to change the bug summary to something more > generic like "Fix native library code which does not return a clean > boolean" and fix the additional occurrences of this antipattern > reported by Matthias in this mail thread: > http://mail.openjdk.java.net/pipermail/jdk-dev/2018-September/thread.html#1986 > I think it would makelittle sense to open a new issue for each of > them. Probably. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
