-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
goto exit
pattern should be replaced with RAII in C++
#7721
Comments
I'm gradually doing a mostly-mechanical removal of simple |
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this issue
Jun 18, 2021
#### Problem The `Exit` group of error-handling macros inhibit scoping, and are unnecessary when there is no cleanup at the `exit:` label. Part of project-chip#7721 _goto exit pattern should be replaced with RAII in C++_ #### Change overview Remove simple `exit: return` from * `src/app` * `src/controller` * `src/crypto` * `src/inet` * `src/lib` * `src/protocols` * `src/setup_payload` * `src/transport` replacing `Exit` macros with corresponding `Return` macros. Note that this does not change any `exit:` blocks with actual cleanup. #### Testing No change to functionality; existing tests should catch regressions.
kpschoedel
added a commit
to kpschoedel/connectedhomeip
that referenced
this issue
Jun 18, 2021
#### Problem The `Exit` group of error-handling macros inhibit scoping, and are unnecessary when there is no cleanup at the `exit:` label. Part of project-chip#7721 _goto exit pattern should be replaced with RAII in C++_ #### Change overview Remove simple `exit: return` from * `src/app` * `src/controller` * `src/crypto` * `src/inet` * `src/lib` * `src/protocols` * `src/setup_payload` * `src/transport` replacing `Exit` macros with corresponding `Return` macros. Note that this does not change any `exit:` blocks with actual cleanup. #### Testing No change to functionality; existing tests should catch regressions.
woody-apple
pushed a commit
that referenced
this issue
Jun 21, 2021
#### Problem The `Exit` group of error-handling macros inhibit scoping, and are unnecessary when there is no cleanup at the `exit:` label. Part of #7721 _goto exit pattern should be replaced with RAII in C++_ #### Change overview Remove simple `exit: return` from * `src/app` * `src/controller` * `src/crypto` * `src/inet` * `src/lib` * `src/protocols` * `src/setup_payload` * `src/transport` replacing `Exit` macros with corresponding `Return` macros. Note that this does not change any `exit:` blocks with actual cleanup. #### Testing No change to functionality; existing tests should catch regressions.
nikita-s-wrk
pushed a commit
to nikita-s-wrk/connectedhomeip
that referenced
this issue
Sep 23, 2021
#### Problem The `Exit` group of error-handling macros inhibit scoping, and are unnecessary when there is no cleanup at the `exit:` label. Part of project-chip#7721 _goto exit pattern should be replaced with RAII in C++_ #### Change overview Remove simple `exit: return` from * `src/app` * `src/controller` * `src/crypto` * `src/inet` * `src/lib` * `src/protocols` * `src/setup_payload` * `src/transport` replacing `Exit` macros with corresponding `Return` macros. Note that this does not change any `exit:` blocks with actual cleanup. #### Testing No change to functionality; existing tests should catch regressions.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This stale issue has been automatically closed. Thank you for your contributions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
there is no functional difference between
ExitNow()
andreturn
or betweenExitNow(err = FOO)
andreturn foo
except that the goto allows a 'separately defined' code to be run under theexit:
label.Using goto has several drawbacks in code organization and readability:
C++ has the ability to do RAII which results in easier 'provable correct' code for things like memory allocation or cleanup.
Proposed Solution
Anything with empty cleanup like
exit: return err
has no benefit from the goto pattern at all. These methods should switch to just 'return' or equivalent macros.Methods that do cleanup should be reviewed for ability to convert to RAII (resource cleanup generally can be RAII, the pattern of 'log if error' is not as clear).
This is not a general ban on
goto exit
, however a strong preference on RAII. Goto exit is a 'I gave up, this cannot be done in C++', which is acceptable but only after RAII was tried.The text was updated successfully, but these errors were encountered: