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

add auto_json_decref macro #279

Closed
wants to merge 1 commit into from

Conversation

sjaeckel
Copy link

gcc and clang implement the compiler attribute cleanup() [1] which lets
you add a function call that is invoked as soon as a variable goes out
of scope.

This feature improves code where you retrieve a json_t object at one
point and you're not interested to keep it over a longer period of time.

[1] https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-g_t_0040code_007bcleanup_007d-variable-attribute-3485

gcc and clang implement the compiler attribute cleanup() [1] which lets
you add a function call that is invoked as soon as a variable goes out
of scope.

This feature improves code where you retrieve a json_t object at one
point and you're not interested to keep it over a longer period of time.

[1] https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-g_t_0040code_007bcleanup_007d-variable-attribute-3485
@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.254% when pulling 9961e4a on sjaeckel:json-autodecref into 006638a on akheron:master.

@Mephistophiles
Copy link
Contributor

Home immediately know all the dangerous moments, such as:

void blah (json_t * test) {
    auto_json_decref json_t * str = json_string ( "test");

    json_object_set_new_nocheck (test, "blah", str);
}

@sjaeckel
Copy link
Author

@Mephistophiles as already said in the commit "...where you retrieve a json_t object at one
point and you're not interested to keep it over a longer period of time." and if you know that json_object_set_new_nocheck() takes ownership of str why would you add a auto_json_decref?

I just want to say: people who'd add a auto_json_decref there, would also just do json_decref(str)

@sjaeckel
Copy link
Author

@akheron is there a chance this will be merged?

@akheron
Copy link
Owner

akheron commented Apr 20, 2016

Yes, there definitely is a chance. My only concern is that this cannot be used in portable code, and that has to be communicated to users very clearly.

It's a shame actually. If all compilers had a way to do this, this should be the default and you would need to use an attribute to disable autodecref.

@sjaeckel
Copy link
Author

My only concern is that this cannot be used in portable code...

That's for sure :-( Therefor it's also enclosed in #ifdef __GNUC__ ...

... and that has to be communicated to users very clearly.

Can I help you with anything there?

It's a shame actually. If all compilers had a way to do this, this should be the default and you would need to use an attribute to disable autodecref.

Yes it's a shame, but who uses a compiler that isn't based on or at least supports most of the functionality of gcc anyways? ...icc supports it and who cares about msvc... ;-)

@akheron
Copy link
Owner

akheron commented Apr 20, 2016

Can I help you with anything there?

You could document it. That's what has to be done in any case :)

@akheron
Copy link
Owner

akheron commented Aug 31, 2016

Superseded by #301

@akheron akheron closed this Aug 31, 2016
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.

None yet

4 participants