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 error reporting #26

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

Conversation

aremmell
Copy link

@aremmell aremmell commented Jun 6, 2018

Thanks for the library. I'm now using it to load and save configuration files in an IoT Linux service.

I have made some small additions I think you will find useful (feel free to disregard if not!):

  1. Add function json_strerror. Takes an int return value from any other function in the library and returns a const char* of the human-readable error message for that code (or '<unknown>' if it isn't a known error code).
  2. Add function json_perror. Calls json_strerror and outputs the message to stderr.

All of my changes can be disabled by uncommenting the #define JSON_NO_ERRORMSG line I added to json.h. I did this in case memory constraints are a concern.

These functions are intended to be in the spirit of the POSIX strerror and perror functions.

Cheers,
Ryan

@aremmell
Copy link
Author

aremmell commented Jun 6, 2018

Another suggested improvement would be to add a global to store the 'last error' for the calling thread, and then remove the int parameter from these functions and instead add a new function called json_geterror.

json.h Outdated
@@ -52,7 +56,8 @@ typedef enum

typedef enum
{
/* SUCCESS = 0 */
/* no error*/
JSON_SUCCESS = 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be consistent, the indentation in the file is tabulation.

@NicolasDP
Copy link
Contributor

I think it is a very useful addition to the library. However, the maintainer of this repo might wish you to replace your 4 space indentation by a tabulation (in both json.c and json.h) to remain consistent.

Also, somehow, the permission settings on the files have been changed from 644 to 744 (with execution rights on it).

@aremmell
Copy link
Author

aremmell commented Jun 6, 2018

Thanks, @NicolasDP

I wrote this in a git repo on an NFS mount on the IoT board hence the permissions (the mount needs to be accessible to world since the remote users aren’t known).

I can/will fix that and the indentation if desired.

Cheers

@aremmell aremmell closed this Jun 6, 2018
@aremmell aremmell reopened this Jun 6, 2018
@aremmell
Copy link
Author

aremmell commented Jun 6, 2018

lol @ me. I did that probably the worst way possible and I think I just caught back up. Never code without food.

Anyway, it turns out some of the formatting was using spaces, so that lead to much confusion on my part trying to read diffs.

I think it should be good now, though. And the permissions should be good as well.

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.

2 participants