Re: [Swig-user] string parameters produce unsafe JNI code

[<sm...@su...>]

----- Original Message ----- 
From: "William S Fulton" <ws...@fu...>
To: "Steve Robbins" <sm...@su...>
Cc: <swi...@li...>
Sent: Friday, November 24, 2006 4:03 PM
Subject: Re: [Swig-user] string parameters produce unsafe JNI code


> Steve Robbins wrote:
>> Howdy,
>>
>> I've been looking at the JNI code produced by SWIG 1.3.29 when wrapping
>> functions that take a char* or const char* parameter.  The result doesn't
>> look safe to me.
>>
>> Here's the interface file:
>>
>> %module test
>>
>> %inline{
>> void func1( const char* x ) {};
>> void func2( char* x ) {};
>> }

[...]

>> Here's what func2() becomes.
>>
>> JNIEXPORT void JNICALL Java_testJNI_func2(JNIEnv *jenv, jclass jcls,
>> jstring jarg1) {
>>    char *arg1 = (char *) 0 ;
>>
>>    (void)jenv;
>>    (void)jcls;
>>    arg1 = 0;
>>    if (jarg1) {
>>      arg1 = (char *)(*jenv)->GetStringUTFChars(jenv, jarg1, 0);
>>      if (!arg1) return ;
>>    }
>>    func2(arg1);
>>    if (arg1) (*jenv)->ReleaseStringUTFChars(jenv, jarg1, arg1);
>> }
>>
>> What alarms me about this is that func2() could try to mutate the string,
>> which is presumably a very bad idea.  Shouldn't the default typemaps
>> be making a copy of the string in order to pass a non-const pointer to
>> func2() rather than casting away constness and praying?
>>
>> Any insight appreciated.
>>
>
> SWIG treats char * as an immutable string and this is documented. This
> is in line with the technically incorrect but common C practice of using
> a char * as an immutable string. There is nothing in the code that will
> not work as intended or that is unsafe. However, it will ignore any
> changes that the function might make to the buffer pointed to by char *.

What alarms me is that GetStringUTFChars() returns a const char*, indicating
that you're not supposed to mutate the string.  That pointer, incidentally, 
may or may
not be a copy of the Java String memory, so any change may or may not be 
ignored.

Personally, if I had meant func2() to take a pointer to an immutable string,
I would have used "const char*" rather than "char *".  Though, as you 
indicate,
not everyone is strict about const correctness and there's a body of code
out there not using const.

> If you don't like this behaviour you can apply alternative typemaps, for
> example by mapping to a StringBuffer or StringBuilder.

Yes.  However, that goes against the principle of Least Surprise -- at least 
for
programmers who are const-aware -- and forces everyone to write the 
alternative
typemaps.  If swig's default type mapping cannot be changed, could the 
standard
swig library at least provide the typemaps and an easy way to use them, e.g. 
a
command line switch or a directive?

Cheers,
-Steve