samples: net: mdns_responder: fix coverity issues #33162
Conversation
Check return value if sendto() fails. Coverity-CID: 215391 Fixes #33093 Signed-off-by: Guðni Már Gilbert <[email protected]>
Avoid resource leak by closing socket when there is an error. Coverity-CID: 215381 Fixes #33094 Signed-off-by: Guðni Már Gilbert <[email protected]>
| welcome(client_fd); | ||
| r = welcome(client_fd); | ||
| if (r == -1) { | ||
| NET_DBG("send() failed (%d)", errno); |
There was a problem hiding this comment.
I'd say this should be NET_ERR().
There was a problem hiding this comment.
@pfalcon Should I also add NET_ERR() when bind(), listen() and socket() fail? They seem to be equally fatal errors.
There was a problem hiding this comment.
Well, I don't know. If you used NET_DBG here because it's used in other similar places in this sample, that's ok. Or can just use NET_ERR here regardless (I'd do that ;-) ). I'm definitely not calling for scope creep in changes ;-).
There was a problem hiding this comment.
Yeah I used NET_DBG() since it's used everywhere else. The main focus is to fix the coverity issue where the return value was not checked. I could add a separate commit to change NET_DBG() to NET_ERR() on top of the current changes.
@jukkar @tbursztyka what are your thoughts on the matter?
There was a problem hiding this comment.
The main focus is to fix the coverity issue
Right, I remember that, and why I say I don't want to introduce more scope creep into that. LGTM.
There was a problem hiding this comment.
Please use NET_DBG() here, and if you want, you can send a separate PR later to change those to NET_ERR().
samples/net/mdns_responder/src/service.csamples/net/mdns_responder/src/service.c