-
Notifications
You must be signed in to change notification settings - Fork 463
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
This fixes access to indirect values within Java destination #1732
This fixes access to indirect values within Java destination #1732
Conversation
This user does not have permission to start the build. Can one of the admins verify this patch and start the build? (admin please type: ok to test) |
@kira-syslogng ok to test |
Build SUCCESS, the tests were executed on test branch: master and test suite: functions |
|
||
(*env)->ReleaseStringUTFChars(env, name, name_str); | ||
|
||
if (value) | ||
{ | ||
return (*env)->NewStringUTF(env, value); | ||
gchar stripped_value[value_len + 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would avoid the allocation of string on the stack, the value_len
could be big.
You could use the g_strndup
instead, with that the \0
termination is guaranteed, although you must g_free
it after thew NewStringUTF
call.
Something like that:
gchar *stripped_value = g_strndup(value, value_len);
jstring java_string = (*env)->NewStringUTF(env, stripped_value);
g_free(stripped_value);
return java_string;
Note: with this the #include <string.h>
not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strndup() can be slow, and this might be a fastpath, although I am not sure how "fast" it is considering it is called from Java.
Anyway, there's a macro called APPEND_ZERO() which checks if the specific name-value pair is zero terminated already, and avoids the copying in that case. Still, that would allocate memory on the stack, which might be limited to couple of megabytes. but if that's the case, the remaining call sites of APPEND_ZERO would be affected as well.
terminated. stick to stack based allocation as macro is used in various modules which would also fail.
Build SUCCESS, the tests were executed on test branch: master and test suite: functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all good, just one small note: it would be nice if you could squash your comments.
Related issue: #1714