Skip to content
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

Support creation of zero-sized primitive Java arrays #85

Merged
merged 2 commits into from
Aug 29, 2022

Conversation

niloc132
Copy link
Contributor

Fixes #84

@@ -1938,11 +1938,11 @@ int JType_ConvertVarArgPyArgToJObjectArg(JNIEnv* jenv, JPy_ParamDescriptor* para
}

itemCount = pyBuffer->len / pyBuffer->itemsize;
if (itemCount <= 0) {
if (itemCount < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming pyBuffer is correct, there shouldn't be any way to trigger this. I wonder if we should prefer to remove the blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from trying to avoid creating zero-length arrays (which apparently work correctly, but perhaps it would be better to only keep references to singleton empty arrays?), I can't imagine why this check would have existed.

It isn't the only size < 0 check in the codebase, so it might still serve as a reasonable sanity check? I could easily go either way, will be guided by your preference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The majority of < 0 checks are against return values where -1 represents an exceptional state.

        itemCount = PySequence_Length(pyArg);
        if (itemCount < 0) {
            return -1;
        }

ie, this returns -1 if pyArg is not a sequence.

The one other extraneous check I could find is:

    *size = PyUnicode_GET_SIZE(pyUnicode);
    if (*size < 0) {
        goto error;
    }

I don't see any cpython documentation that this is expected to ever trigger - the assumption is that pyUnicode is a unicode object; and the code base does make that check.

As such, I think it makes sense to remove these exception checking blocks.

@devinrsmith devinrsmith self-requested a review August 29, 2022 18:20
@devinrsmith devinrsmith merged commit e1c13bd into jpy-consortium:master Aug 29, 2022
niloc132 added a commit to deephaven/deephaven-core that referenced this pull request Sep 1, 2022
)

Reworks how we handle logging between Java and Python, giving Python code
more control over and more visibility into the stdout and stderr streams.
These changes also make it possible to see the same logs in both the web UI
and the python terminal, in the case of DHaaL from Python.

Follow-up is required to address some inefficiency around PyObject's tools
for calling methods, see #2793.

For #2723, jpy-consortium/jpy#85 is also required, to deal with zero-length
bytestrings.

Fixes #2734
Partial #2723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jpy should be able to create zero-sized arrays
2 participants