[Bug 56878] Checking whether unsigned int is less than zero is useless

[bu...@apache.org]

https://issues.apache.org/bugzilla/show_bug.cgi?id=56878

--- Comment #1 from Konstantin Kolinko <kn...@gmail.com> ---
(In reply to Christopher Schultz from comment #0)
> I just saw this when compiling with clang on Mac OS X (partially because
> clang shows all warnings and errors in BIG BOLD SUPER HIGHLIGHTED TEXT!):
> 
> src/jnilib.c:130:11: warning: comparison of unsigned expression < 0 is always
>       false [-Wtautological-compare]
>     if (l < 0)
>         ~ ^ ~
> 1 warning generated.
> 
> I checked, and the function [jstring tcn_new_stringn(JNIEnv *env, const char
> *str, size_t l)] accepts an argument of size_t and then checks to see if it
> is less than 0. Since it's unsigned, it will never be negative.

What header defines size_t ? Is it platform-dependent?

Passing "-1" as a size might be a valid use case (in theory).

BTW, the "len" local variable is cast to (jsize) on one line and to (jint) on
another.

> Looking at similar functions, I can see that tcn_new_stringn actually checks
> the Java runtime to see if new references can be created in the local scope
> and fails if it can't. The other similar functions do not do such checking,
> and are therefore fragile.

The only other function that calls DeleteLocalRef there (tcn_get_string) has
such check. I do not think anything is fragile there.

Documentation for "EnsureLocalCapacity" [1] says that JVM does not enforce the
limit, but allocates references beyond the ensured capacity.

I think the EnsureLocalCapacity call here just makes it to produce a nice
explicit OutOfMemoryError instead of a NULL return value. I think that it
wouldn't become fragile if EnsureLocalCapacity call is omitted, because on the
following lines all function that allocate objects are checked for returning a
NULL value.

BTW, comparing "tcn_new_stringn" and "tcn_new_string" implementations,
there is subtle difference between them:
- the "tcn_new_string" method calls "NewStringUTF" (so it uses "modified" UTF-8
used by Java),
- the "tcn_new_stringn" method copies C chars into a byte array and uses
platform encoding to convert them into Java characters.

What approach is the correct one depends on where the char* data came from.

[1]
http://docs.oracle.com/javase/1.5.0/docs/guide/jni/spec/functions.html#global_local
[2] http://stackoverflow.com/questions/7081853/jni-ensurelocalcapacity-why

-- 
You are receiving this mail because:
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org