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

fix: some bugs in detach and replace #456

Merged
merged 3 commits into from
May 6, 2020
Merged

fix: some bugs in detach and replace #456

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3999b12
feat: set list head's prev in parse_array and parse_object
miaoerduo Apr 3, 2020
a65abf2
fix: error list head's prev when detach the last item
miaoerduo Apr 3, 2020
cb4661c
fix: errors in replacing the first item when array_size is 1, and rep…
miaoerduo Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cJSON.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,10 @@ static cJSON_bool parse_array(cJSON * const item, parse_buffer * const input_buf
success:
input_buffer->depth--;

if (head != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@miaoerduo thanks your suggestion! but this change make the testcase fail, I have to make some changes for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the 0th is unnecessary? How about the others? I have test these in my project. :P

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, it's needed. compiling it with option cmake ENABLE_VALGRIND=On .. when make check, memory check is very important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I have commited with company's email.
So I have to changed the email in commits and force pushed again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May my modification be merged into this project?
Is there any other thing I could do?
I haven't contributed to a project with so many stars!

Copy link
Collaborator

Choose a reason for hiding this comment

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

This version is released no more than 24 hours since you make this contribution, you have done a good job :), before I merge it, I need some time to make sure it won't introduce any problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow! Thank YOU very much!

head->prev = current_item;
}

item->type = cJSON_Array;
item->child = head;

Expand Down Expand Up @@ -1681,6 +1685,10 @@ static cJSON_bool parse_object(cJSON * const item, parse_buffer * const input_bu
success:
input_buffer->depth--;

if (head != NULL) {
head->prev = current_item;
}

item->type = cJSON_Object;
item->child = head;

Expand Down Expand Up @@ -2202,6 +2210,12 @@ CJSON_PUBLIC(cJSON *) cJSON_DetachItemViaPointer(cJSON *parent, cJSON * const it
/* first element */
parent->child = item->next;
}
else if (item->next == NULL)
{
/* last element */
parent->child->prev = item->prev;
}

/* make sure the detached item doesn't point anywhere anymore */
item->prev = NULL;
item->next = NULL;
Expand Down Expand Up @@ -2299,6 +2313,10 @@ CJSON_PUBLIC(cJSON_bool) cJSON_ReplaceItemViaPointer(cJSON * const parent, cJSON
}
if (parent->child == item)
{
if (parent->child->prev == parent->child)
{
replacement->prev = replacement;
}
parent->child = replacement;
}
else
Expand All @@ -2310,6 +2328,10 @@ CJSON_PUBLIC(cJSON_bool) cJSON_ReplaceItemViaPointer(cJSON * const parent, cJSON
{
replacement->prev->next = replacement;
}
if (replacement->next == NULL)
{
parent->child->prev = replacement;
}
}

item->next = NULL;
Expand Down