tezc / sc

Common libraries and data structures for C.
BSD 3-Clause "New" or "Revised" License
2.23k stars 239 forks source link

Fix error propagation #86

Closed svladykin closed 2 years ago

svladykin commented 2 years ago

errno is not set at this point, so we see either some previous error or no error at all

codecov[bot] commented 2 years ago

Codecov Report

Merging #86 (a74c659) into master (3085b27) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #86   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          21       21           
  Lines        2185     2188    +3     
  Branches      382      383    +1     
=======================================
+ Hits         2181     2184    +3     
  Partials        4        4           
Impacted Files Coverage Δ
socket/sc_sock.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3085b27...a74c659. Read the comment docs.

tezc commented 2 years ago

@svladykin So, you see getsockopt() returns 0 but ret is not zero. This is a bit weird, is this Windows? It feels like a bug in the API. Documented behavior says it will return -1 on error.

RETURN VALUE         [top](https://man7.org/linux/man-pages/man2/getsockopt.2.html#top_of_page)
       On success, zero is returned for the standard options.  On error,
       -1 is returned, and [errno](https://man7.org/linux/man-pages/man3/errno.3.html) is set to indicate the error.
svladykin commented 2 years ago

It is Linux, but the API looks fine to me: 0 is returned when the getsockopt call itself is successful. If rc is 0 we know that we successfully got SO_ERROR socket option value into ret and can check if anything is wrong with the socket by looking at ret. If getsockopt returned -1 it means that we were not even able to get the socket option value, so the socket is probably broken.

tezc commented 2 years ago

Thanks