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

Check that all functions that steal a value decref it on error #135

Closed
akheron opened this issue Aug 28, 2013 · 4 comments
Closed

Check that all functions that steal a value decref it on error #135

akheron opened this issue Aug 28, 2013 · 4 comments

Comments

@akheron
Copy link
Owner

akheron commented Aug 28, 2013

No description provided.

@akheron
Copy link
Owner Author

akheron commented Aug 28, 2013

See #63 (comment)

@fabled
Copy link

fabled commented Jan 18, 2017

This is probably most important for json_pack(). When you have multiple 'o' specifiers, and an error occurs, it's impossible for the caller to know which references where stolen and which not. The function should probably always steal the references (e.g. explicitly call decref on the remainder of 'o' specifiers not already stolen).

@akheron
Copy link
Owner Author

akheron commented Jan 20, 2017

Good point! Would you like to write a patch?

@coreyfarrell
Copy link
Collaborator

@fabled I posted a patch for json_pack. Sorry if this is a duplicate message, I'm not sure if github notifies issue participants of related pull requests.

Mephistophiles pushed a commit to Mephistophiles/jansson that referenced this issue Feb 12, 2018
Users of the "o" format have an expectation that the object reference
will be stolen.  Any error causes the collection process to end early.
This patch causes json_pack and related functions to continue scanning
the format and parameters so all references can be stolen to prevent
leaks.  This makes no attempt to continue processing if the format
string is broken or missing.

'make check' still passes.  Ran test_pack under valgrind and verified
that the leaked reference is fixed. Added a test which uses refcounts
to verify that the reference was correctly stolen after a NULL value
error.

Issue akheron#135
Mephistophiles pushed a commit to Mephistophiles/jansson that referenced this issue Feb 12, 2018
This function needs to release a reference to value if the other
arguments are invalid.

Issue akheron#135
Mephistophiles pushed a commit to Mephistophiles/jansson that referenced this issue Feb 12, 2018
The `O` format causes reference counts to increase, but in an error they
are not released.  Callers to unpack functions that use the `O` format
should use pointers pre-initialized to NULL so they can safely release
the reference on error.

Also corrected typo which said this was like `O` (itself).

Fixes akheron#135
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

No branches or pull requests

3 participants