Fix faulty io net error handling code in posix BSP #101
Conversation
Basically, when the SDK allocates the structure in which it will store the socket descriptor it opens when connecting to the iotcore server, it initializes it to 0. If during the process of creating the socket, the posix BSP routine (iotc_bsp_io_net_socket_connect) that creates the socket can’t resolve the iotcore server name, it returns an error (leaving the socket descriptor field initialized to 0). In turn, the io net routine (iotc_io_net_layer_connect) that itself called iotc_bsp_io_net_socket_connect, on seeing the error, will then execute its error handling code that itself ends up calling the io net routine iotc_io_net_layer_close_externally that then calls the posix BSP iotc_bsp_io_net_close_socket routine. The posix BSP iotc_bsp_io_net_close_socket routine then blindly closes socket descriptor 0 (since this is what the data structure tracking the socket descriptor is still initialized to since no socket was ever successfully opened). Files modified: src/bsp/platform/posix/iotc_bsp_io_net_posix.c - always set *iotc_socket to -1 before attempting to connect or after closing the socket and always check that *iotc_socket is not -1 before attempting to close it in iotc_bsp_io_net_close_socket().
|
Hey @pnfisher can you make a no-op change to nudge the CI |
|
Yes, will try to get to this today. |
pnfisher
force-pushed
the
posix_stdinclosefix
branch
from
4a0a929 to
a429c2c
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
@galz10 So I did a Also, for some reason the google bot seems to have decided my CLAs are no longer signed, even though it was okay with them on May 2nd when I first submitted this PR. Not sure what the problem is. Can someone on your end resolve this? |
|
Yea that’s what I meant , I don’t know why the CLA bot did that , did you commit to the branch using another email that you didn’t sign the CLA with ? Also if you go to the link does it say you’ve already signed a CLA ? |
|
No, I sued the email I always use. And the same I used for my original PR. |
|
I think you just need to comment |
|
@googlebot I consent. |
|
@galz10 Any ideas on how we can get this fix unblocked? |
|
@pnfisher I'm working with one of the engineers to figure out the problem with Travis CI, I'll ask around to see what the problem is with the CLA |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
Hey @pnfisher everything is correct on the CLA, i'm still trying to figure out what's wrong with the Travis CI test not initializing |
|
@googlebot I consent |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
|
@gguuss Just wondering if there are any updates concerning this PR. Is it likely ever to be merged, or are things so messed up that we should just kill this PR and start again from scratch? |
4 participants
Basically, when the SDK allocates the structure in which it will
store the socket descriptor it opens when connecting to the iotcore
server, it initializes it to 0. If during the process of creating the
socket, the posix BSP routine (iotc_bsp_io_net_socket_connect) that
creates the socket can’t resolve the iotcore server name, it returns
an error (leaving the socket descriptor field initialized to 0). In
turn, the io net routine (iotc_io_net_layer_connect) that itself
called iotc_bsp_io_net_socket_connect, on seeing the error, will then
execute its error handling code that itself ends up calling the io
net routine iotc_io_net_layer_close_externally that then calls the
posix BSP iotc_bsp_io_net_close_socket routine. The posix BSP
iotc_bsp_io_net_close_socket routine then blindly closes socket
descriptor 0 (since this is what the data structure tracking the
socket descriptor is still initialized to since no socket was ever
successfully opened).
Files modified:
src/bsp/platform/posix/iotc_bsp_io_net_posix.c
closing the socket and always check that *iotc_socket is not -1
before attempting to close it in iotc_bsp_io_net_close_socket().
#98