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

Improve out-of-memory handling #68

Closed
firepick1 opened this issue Apr 26, 2015 · 7 comments
Closed

Improve out-of-memory handling #68

firepick1 opened this issue Apr 26, 2015 · 7 comments
Assignees
Labels
bug v5 ArduinoJson 5

Comments

@firepick1
Copy link

Benoit, I made a few tweaks in my branch to handle several out-of-memory edge cases that were blowing up my code. You might be interested in this:
firepick1@bc8c3d9

@bblanchon
Copy link
Owner

What happened before you added the changes?
It should be 100% safe to manipulate invalid JsonVariant, JsonArray and JsonObject, every operation should be nops.

@bblanchon bblanchon added the bug label Apr 26, 2015
@bblanchon
Copy link
Owner

PS: Do you think you can write a unit test?

@bblanchon bblanchon self-assigned this Apr 26, 2015
@firepick1
Copy link
Author

I've got a unit test in my code that fails without these changes. I'll try
to add one in ArduinoJson.

It was failing on something like this:
StaticJsonBuffer<JSON_OBJECT_SIZE(4)> jbtiny;
{"a":{"b":{"c":{"d":123}}}

jbtiny.parseObject() returns success.
However, a printTo() of the parsed JSON yields:
{"a":{"b":{"c":{}}

I knew that your intent was to make the invariants mutable, but I couldn't
figure out why this particular case broke, so after getting a headache I
slapped a goto bandaid on it and it seemed to work.

On Sun, Apr 26, 2015 at 7:49 AM, Benoît Blanchon notifications@github.com
wrote:

PS: Do you think you can write a unit test?


Reply to this email directly or view it on GitHub
#68 (comment)
.

Karl Lew
FIREPICK SERVICES LLC
karl@firepick.org
www.firepick.org

@bblanchon
Copy link
Owner

Excellent!
I'll fix this ASAP.

@firepick1
Copy link
Author

Cool. Thanks! I was trying to figure out the unit tests and that would take
some time.

On Sun, Apr 26, 2015 at 8:08 AM, Benoît Blanchon notifications@github.com
wrote:

Excellent!
I'll fix this ASAP.


Reply to this email directly or view it on GitHub
#68 (comment)
.

Karl Lew
FIREPICK SERVICES LLC
karl@firepick.org
www.firepick.org

@bblanchon
Copy link
Owner

I fixed the bug in a completely different way.
Please have a look at branch issue68.
I'll merge into master ASAP.

@firepick1
Copy link
Author

Thanks, Benoit!

Your fix feels much better than my bandaid. :D

On Sun, Apr 26, 2015 at 11:01 AM, Benoît Blanchon notifications@github.com
wrote:

I fixed the bug in a completely different way.
Please have a look at branch issue68
https://github.com/bblanchon/ArduinoJson/commits/issue68.
I'll merge into master ASAP.


Reply to this email directly or view it on GitHub
#68 (comment)
.

Karl Lew
FIREPICK SERVICES LLC
karl@firepick.org
www.firepick.org

bblanchon pushed a commit that referenced this issue Apr 27, 2015
bblanchon pushed a commit that referenced this issue Apr 27, 2015
Repository owner locked and limited conversation to collaborators Sep 21, 2018
@bblanchon bblanchon added the v5 ArduinoJson 5 label Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug v5 ArduinoJson 5
Projects
None yet
Development

No branches or pull requests

2 participants