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

Copy object item key to replacement in ReplaceItemViaPointer #859

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

oantby
Copy link

@oantby oantby commented May 18, 2024

README indicates that object items can be replaced with cJSON_ReplaceItemViaPointer. However, the key for an object item will be lost in that process, unless the caller uses information on cJSON internals to update the replacement object's key with that of the source. cJSON_ReplaceItemInObject is listed as the other method for replacing an object item, and does set the key on the replacement value.
This PR updates cJSON_ReplaceItemViaPointer to copy any string on the item to be replaced with that of the source.
Consideration was made to just reuse the string from the item being replaced - opted to effectively replicate the existing functionality of cJSON_ReplaceItemInObject, which always allocated a new string for key.

Example:

	cJSON *j, *r, *t;
	j = cJSON_Parse("{\"foo\": \"bar\"}");
	r = cJSON_Parse("{\"bar\": true}");
	
	t = cJSON_GetObjectItemCaseSensitive(j, "foo");
	cJSON_ReplaceItemViaPointer(j, t, r);
	puts(cJSON_Print(j));

Under master, this prints:

{
	"":	{
		"bar":	true
	}
}

After this PR:

{
	"foo":	{
		"bar":	true
	}
}

Couldn't find contributor guidelines at a glance, so let me know if there are any other specifics needed here.

@oantby
Copy link
Author

oantby commented May 19, 2024

I found the contributing guidelines, and added this to the unit test for object item replacement.

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.

1 participant