-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[RUNTIME]crt error handling #5147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siju-samuel Thanks for bringing the improvements. I think we still need to follow MISRA-C rules as closely as possible. Specifically, we should have only one return statement in a function.
src/runtime/crt/graph_runtime.c
Outdated
reader->BeginArray(reader); | ||
if (!(reader->NextArrayItem(reader))) { | ||
fprintf(stderr, "invalid json format: failed to parse `node_id`\n"); | ||
return status; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MISRA-C requires having only one return statement in a function.
src/runtime/crt/graph_runtime.c
Outdated
@@ -38,25 +38,28 @@ uint32_t Shape_Accumulate(int64_t * shape, uint32_t ndim) { | |||
} | |||
|
|||
int NodeEntry_Load(TVMGraphRuntimeNodeEntry * entry, JSONReader * reader) { | |||
int status = 0; | |||
int status = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is intended to be success, and set status to error when we are in trouble. Subsequently, return status at the end of the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @siju-samuel for the changes |
Thanks @liangfu @siju-samuel the PR has been merged. |
* crt error handling * Review comments fixed
* crt error handling * Review comments fixed
@liangfu @tmoreau89 Please help to review. TIA
Most of the places error was not returned to caller and the function will continue to execute and cause some exception. This is handled.
Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.