Skip to content

json: don't copy strings while escaping#76

Merged
nashif merged 2 commits intozephyrproject-rtos:masterfrom
mbolivar:json-escape-inplace
May 8, 2017
Merged

json: don't copy strings while escaping#76
nashif merged 2 commits intozephyrproject-rtos:masterfrom
mbolivar:json-escape-inplace

Conversation

@mbolivar
Copy link
Copy Markdown
Contributor

@mbolivar mbolivar commented May 3, 2017

Re-implement json_escape() with a version that doesn't make a copy of the input string. Add test cases.

@nashif nashif self-assigned this May 3, 2017
@nashif nashif requested a review from lpereira May 3, 2017 23:37
Copy link
Copy Markdown
Member

@lpereira lpereira left a comment

Choose a reason for hiding this comment

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

LGTM with a nitpick.

lib/json/json.c Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nitpick, but could be a for (; next != str; next--) instead. Can remove the last few lines in the loop body.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried and failed to think of a way to do it without while(1) / break. The edge cases are a little tricky.

The way you suggest won't work if *len == 1 and the character at str[0] needs escaping, for example.

Marti Bolivar added 2 commits May 4, 2017 09:47
Currently, json_escape() allocates a temporary buffer on the stack
that's the size of the string being escaped.

Stack space is precious and longer JSON strings can run into the
hundreds of bytes, so re-implement this routine to escape in place.

Signed-off-by: Marti Bolivar <marti.bolivar@linaro.org>
These all pass.

Signed-off-by: Marti Bolivar <marti.bolivar@linaro.org>
@mbolivar
Copy link
Copy Markdown
Contributor Author

mbolivar commented May 4, 2017

Update:

  • rewrote as a for loop
  • added a test case to catch the edge condition discussed in last night's comments

@nashif nashif merged commit 88f33dc into zephyrproject-rtos:master May 8, 2017
@mbolivar mbolivar deleted the json-escape-inplace branch May 18, 2017 04:12
nashif pushed a commit that referenced this pull request Jun 29, 2021
- to get the update hal_nxp from PR #76

Signed-off-by: Lucien Zhao <lucien.zhao@nxp.com>
CristXu pushed a commit that referenced this pull request Jul 4, 2021
- to get the update hal_nxp from PR #76

Signed-off-by: Lucien Zhao <lucien.zhao@nxp.com>
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.

4 participants