You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2021/02/26 08:36:29 UTC
[GitHub] [mynewt-mcumgr] KKopyscinski opened a new pull request #118: img_mgmt_state_set_pending: remove unnecessary rc assigns
KKopyscinski opened a new pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118
rc assigns were leftover of state before adding `done:` marker. They
were not used as actual return values no more, as first thing that
happens at done marker is overwrite of them
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] KKopyscinski commented on pull request #118: img_mgmt_state_set_pending: bail out if errors occur
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#issuecomment-799249071
So maybe just not overwrite error from `img_mgmt_impl_write_pending` and straight out return it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on a change in pull request #118: img_mgmt_state_set_pending: bail out if errors occur
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#discussion_r594157565
##########
File path: cmd/img_mgmt/src/img_mgmt_state.c
##########
@@ -133,15 +133,15 @@ img_mgmt_state_set_pending(int slot, int permanent)
*/
if (state_flags & IMG_MGMT_STATE_F_CONFIRMED && slot != 0) {
rc = MGMT_ERR_EBADSTATE;
- goto done;
+ return rc;
Review comment:
Fix the indentations.
##########
File path: cmd/img_mgmt/src/img_mgmt_state.c
##########
@@ -133,15 +133,15 @@ img_mgmt_state_set_pending(int slot, int permanent)
*/
if (state_flags & IMG_MGMT_STATE_F_CONFIRMED && slot != 0) {
rc = MGMT_ERR_EBADSTATE;
- goto done;
+ return rc;
Review comment:
Fix the indentations, in both cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] KKopyscinski commented on pull request #118: img_mgmt_state_set_pending: remove unnecessary rc assigns
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#issuecomment-799244691
@sjanc After looking at it again, I agree :) And this change makes `done` marker obsolete
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] sjanc merged pull request #118: img_mgmt_state_set_pending: corrected error handling
Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #118: img_mgmt_state_set_pending: bail out if errors occur
Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#issuecomment-799254718
>
>
> So maybe just not overwrite error from `img_mgmt_impl_write_pending` and straight out return it?
I think that should be ok. Better change it though by testing "bad scenario": enforce fail at both points and observe how mcumgr behaves.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] sjanc commented on pull request #118: img_mgmt_state_set_pending: remove unnecessary rc assigns
Posted by GitBox <gi...@apache.org>.
sjanc commented on pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#issuecomment-799233008
Are you sure we should just ignore return value from write_pending() and not bail out instead?
@utzig @de-nordic thoughts?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] KKopyscinski commented on pull request #118: img_mgmt_state_set_pending: bail out if errors occur
Posted by GitBox <gi...@apache.org>.
KKopyscinski commented on pull request #118:
URL: https://github.com/apache/mynewt-mcumgr/pull/118#issuecomment-799307428
@de-nordic I changed the code to what I think is inteded behavior
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org