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.

Sign up for GitHub

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

Jump to bottom

Fix issues with json-c v0.13 (Fixes #1355) #1517

Closed
wants to merge 3 commits into from
Closed

Fix issues with json-c v0.13 (Fixes #1355) #1517

wants to merge 3 commits into from

Conversation

besser82
Copy link

@besser82 besser82 commented Dec 14, 2017 •
edited
Loading

This superseeds #1439 and fixes #1355.

@besser82 besser82 changed the title Fix issues with json-c v0.13 Fix issues with json-c v0.13 (Fixes #1355) Dec 14, 2017
@besser82 besser82 mentioned this pull request Dec 14, 2017
Closed
ddevault
ddevault reviewed Dec 14, 2017
View reviewed changes
sway/ipc-server.c Outdated
// to another json_object, it must be done with a wrapped call to
// json_object_get(). Otherwise the object's refcount does not
// get properly incremented and it will be deleted together with
// the object it got added to.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment belongs in the commit message

@ddevault
Copy link
Contributor

I hate that we have to do this terrible workaround, but I don't see a better way. This sucks.

@besser82
Copy link
Author

@SirCmpwn This is actually NOT a workaround, but well documented behaviour of json_object_object_add() in the json-c API…

See: https://github.com/json-c/json-c/blob/master/json_object.h#L379

Remember, when using
json_object_object_add or json_object_array_put_idx, ownership will
transfer to the object/array.  Call json_object_get if you want to maintain
shared ownership or also add this object as a child of multiple objects or
arrays.  Any ownerships you acquired but did not transfer must be released
through json_object_put.

and: https://github.com/json-c/json-c/blob/master/json_object.h#L408

The reference count will *not* be incremented. This is to make adding
fields to objects in code more compact. If you want to retain a reference
to an added object, independent of the lifetime of obj, you must wrap the
passed object with json_object_get.

Upon calling this, the ownership of val transfers to obj.  Thus you must
make sure that you do in fact have ownership over this object.

@ddevault
Copy link
Contributor

I'm referring to the typedef and the version check header, not the refcounting.

@besser82
ipc-server: Acquire ownership of referenced json_object properly
8272a9b 
When adding a referenced json_object with an unknown lifetime to
another json_object, it must be done with a wrapped call to
json_object_get() to acquire the ownership of that json_object.
@besser82
Adaptions for API change in json-c v0.13
02da9c4
@besser82
Copy link
Author

Ahh, I see… Well, the typedef is not needed, if we remove -Werror… Converting from size_t to int is safe in all cases, since there are no json_array_objects bigger than 4 GiB used somewhere…

@ddevault
Copy link
Contributor

Nah, I prefer this to dropping Werror, since Werror is useful beyond this one json-c problem.

@ddevault
Copy link
Contributor

This has been merged into the 0.15 branch and will make the next bugfix release. Thanks!

@ddevault ddevault closed this Dec 17, 2017
@besser82
Copy link
Author

@SirCmpwn What about the master-branch?

@ddevault
Copy link
Contributor

Don't worry about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddevault ddevault ddevault left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problems with git master versions of json-c
2 participants
@besser82 @ddevault