Azure / azure-c-shared-utility

Azure C SDKs common code
Other
111 stars 203 forks source link

patch to suppress sigpipe #479

Closed fz-lyu closed 3 years ago

fz-lyu commented 3 years ago

patch to suppress sigpipe

https://github.com/Azure/azure-iot-sdk-c/issues/1423

fz-lyu commented 3 years ago

for patch#11 discussed in the last meeting

cc @suhuruli @momuno

ewertons commented 3 years ago

This change is good to go, with it just SIGPIPE is suppressed. Source: https://www.man7.org/linux/man-pages/man2/send.2.html

Since we have a function in the tlsio for suppressing SIGPIPE, this commit wouldn't change the current expected behavior.

ewertons commented 3 years ago

/azp run all

azure-pipelines[bot] commented 3 years ago
No pipelines are associated with this pull request.
ewertons commented 3 years ago

/azp run integrate-into-repo-C-shared-utility

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
ewertons commented 3 years ago

@fanzhe98 , this change fails to build on OS-X (it uses the same socket adapter). For addressing that, one way is to add a define at the top of socketio_berkeley.c:

// MSG_NOSIGNAL is not defined on all platforms where socketio_berkeley.c is used (e.g., OS-X).
// Setting MSG_NOSIGNAL to 0 results in not suppressing SIGPIPE.
#ifndef MSG_NOSIGNAL
#define MSG_NOSIGNAL 0 
#endif

If you do need to have the signal suppressed on OS-X as well, this would be a recourse to be added: https://stackoverflow.com/questions/19509348/sigpipe-osx-and-disconnected-sockets

fz-lyu commented 3 years ago

@ewertons we do not support development on Os-X yet.

I will go ahead and add the lines of ifdef later today

fz-lyu commented 3 years ago

/azp run integrate-into-repo-C-shared-utility

azure-pipelines[bot] commented 3 years ago
Commenter does not have sufficient privileges for PR 479 in repo Azure/azure-c-shared-utility
fz-lyu commented 3 years ago

code updated @ewertons

momuno commented 3 years ago

/azp run integrate-into-repo-C-shared-utility

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
momuno commented 3 years ago

/azp run integrate-into-repo-C-shared-utility

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
momuno commented 3 years ago

/azp run integrate-into-repo-C-shared-utility

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).