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

Guard against PyNULL in arguments #552

Closed
wants to merge 2 commits into from
Closed

Conversation

tkf
Copy link
Member

@tkf tkf commented Sep 1, 2018

fixes (the easier part of) #551

@@ -597,8 +597,16 @@ end
function PyObject(d::AbstractDict)
o = PyObject(@pycheckn ccall((@pysym :PyDict_New), PyPtr, ()))
for k in keys(d)
pykey = PyObject(k)
pyval = PyObject(d[k])
Copy link
Member

Choose a reason for hiding this comment

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

Would be cleaner to have a function

function valid_pyobject(x)
    o = PyObject(x)
    ispynull(o) && error("invalid PyNULL object created from $x")
    return o
end

and call that in cases where we want to check for NULL results.

@stevengj
Copy link
Member

stevengj commented Sep 1, 2018

An unexpected NULL object will generally indicate a bug somewhere else, as is the case here (#553), so I'm not sure it's worthwhile to insert these argument checks everywhere.

@tkf
Copy link
Member Author

tkf commented Sep 1, 2018

I saw ispynull(o) in pystring so I thought calling it makes sense. But I guess you have to use it there, as unless it's impossible to have PyNULL in REPL.

Anyway, I'm OK with not adding this support. Closing it.

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.

2 participants