On 3/22/14 1:38 AM, Mike Duigou wrote:
What if we just abandon adding an IS_TRUE / TO_JBOOLEAN macro for now.

I would like to address adding (jboolean) casts to jni.h which I suspect would 
help with the static analysis issues encountered.
The problem, which I try to solve occurs, when we try to cast some things like BOOL(int32) to jboolean(int8) and we have a chance to invert result from true to false. I am not sure, that I understand how new (jboolean) cast in jni.h solve the problem? Parfait does not warn about assignments from JNI_TRUE to jboolean, so I think it smart enough
For now just use the explicit ? JNI_TRUE : JNI_FALSE whenever a jboolean value 
is needed and we can refactor the naming of the macro, if any is required, 
later.
Sure, but I don't want to copy this across the files, and want to use macro at least. The question is where to place it.

Mike

On Mar 21 2014, at 08:10 , roger riggs <roger.ri...@oracle.com> wrote:

Hi Sergey,

I didn't see any examples of the use case in this thread.  The proposed name 
seems bulky and clumsy.

In the snippet provided, it did not seem necessary to force the cast to 
jboolean.
Both JNI_TRUE and JNI_FALSE are untyped constants.

The macro would just as useful (if I understand the cases) without the cast.

How useful is a simple definition as:

#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)

then it would look ok to see these in sources:

      return IS_TRUE(obj);

      if (IS_TRUE(obj)) {....}

      jboolean ret = IS_TRUE(obj);

The general purpose usage matches the general C conventions for
true and false and match the JNI semantics.

Roger



On 3/19/2014 7:43 PM, Sergey Bylokhov wrote:
Thanks.
So if nobody objects, the final version will be:

#define IS_NULL(obj) ((obj) == NULL)
#define JNU_IsNull(env,obj) ((obj) == NULL)
+#define TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)


On 3/20/14 12:07 AM, roger riggs wrote:
Hi,

Well...  When JNU_RETURN was added, there was a long discussion about
NOT using JNU unless the JNI environment was an argument to the macro.

So, the simple name is preferred.

Roger

On 3/19/2014 4:08 PM, Phil Race wrote:
PS .. so maybe whilst you are touching this file you could do
#define JNU_CHECK_NULL CHECK_NULL
#define JNU_CHECK_NULL_RETURN CHECK_NULL_RETURN

so we could migrate to these (clearer) ones

-phil.

On 3/19/2014 1:05 PM, Phil Race wrote:
I think having it start with "JNU_" is a good suggestion.
I've been wondering over the last week or so if it would not have been
better to have CHECK_NULL called JNU_CHECK_NULL to reduce collision chances
and to make it clearer where it comes from ..

-phil.

On 3/19/2014 12:49 PM, Mike Duigou wrote:
Definitely a useful macro.

I too would prefer a name like TO_JBOOLEAN since it reveals the result type. 
Also all uppercase to identify it as a macro. If we are paranoid and want to 
reduce the chance of a name collision then JNU_TO_JBOOLEAN perhaps.

I would also define the macro as:

#define JNU_TO_JBOOLEAN(obj) (jboolean) ((obj) ? JNI_TRUE : JNI_FALSE)

so that the type of the result is explicit. Unfortunately jni.h doesn't define 
JNI_TRUE or false with a cast to jboolean as they probably should.

Mike

On Mar 19 2014, at 11:36 , Sergey Bylokhov <sergey.bylok...@oracle.com> wrote:

Thanks Anthony!

Can somebody from the core-libs team take a look?

On 3/17/14 10:31 PM, Anthony Petrov wrote:
Personally, I'd call it to_jboolean(obj), but IS_TRUE(obj) sounds good to me 
too. Either way, I'm fine with the fix.

--
best regards,
Anthony

On 3/17/2014 7:01 PM, Sergey Bylokhov wrote:
Hello.
This review request is for the new macro, which simplify conversion to
jboolean. It will be useful for fixing parfait warnings.

We have a lot of places, where we cast some type to jboolean:

BOOL = retVal;
return (jboolean) retVal;

WARNING: Expecting value of JNI primitive type jboolean: mismatched
value retVal with size 32 bits, retVal used for conversion to int8 in return


+++ b/src/share/native/common/jni_util.h    Mon Mar 17 18:28:48 2014 +0400
@@ -277,6 +277,7 @@

  #define IS_NULL(obj) ((obj) == NULL)
  #define JNU_IsNull(env,obj) ((obj) == NULL)
+#define IS_TRUE(obj) ((obj) ? JNI_TRUE : JNI_FALSE)

I am not sure about the name, probably someone have a better suggestion?

The fix is for jdk9/dev.

--
Best regards, Sergey.

--
Best regards, Sergey.




--
Best regards, Sergey.

Reply via email to