squid-cache
Improve handling of xrecvfrom() errors
Changed comm_udp_recvfrom() API to reduce the number of caller error handling bugs. Use std::expected-like API to better distinguish access to metadata associated with a successful xrecvfrom() result from access to errno associated with that call failure. This API change reduces the probability of "forgot to check for errors before accessing positive results" bugs similar to those fixed in 2013 commit cbd5aee3. Several such bugs were fixed in this upgrade as well:
-
comm_udp_recvfrom() was logging (at level 8) bogus
fromaddresses before attempting to receive any bytes. -
Reduced debugs() code duplication across comm_udp_recvfrom() callers by providing level-3+ debugging inside comm_udp_recvfrom().
-
htcpRecv() was logging (at level 3) bogus "from" addresses on errors. htcpHandleMsg() was doing that as well (at level 2).
-
htcpRecv() was counting I/O errors (but not zero-length reads) in "Number of HTCP messages received" counter (i.e.
htcp_pkts_recv). Now HTCP mimics ICP code (i.e.icp.pkts_recv) that only counts successful reads, excluding truncated ones. -
IcmpSquid::Recv() was logging (at level 1) bogus zero "opcode" values and "from" addresses on EAGAIN I/O errors.
-
snmpHandleUdp() was mistreating zero-length reads as I/O errors (that set errno). They are now reported as zero-length reads. Perhaps they should not be reported (or otherwise treated specially) at all.
-
wccpHandleUdp() was reporting unread wccp_i_see_you members on errors if level-3+ debugging was enabled. It still reports and uses those (zero-initialized) unread members after a successful read of fewer than expected bytes; that bug is now explicitly marked with an XXX.
The new Comm::ReceiveFrom() API also makes existing comm_udp_recv() variation unnecessary as callers can simply ignore the address part of the returned metadata.
Also removed idnsRead() BUG BYPASS code added in 2007 commit cc192b50. Back then, IPAddress code was very different from current Ip::Address code. Whatever Squid bug that bypass was working around then is probably gone now. If it is not, we should welcome its exposure and a proper fix!
Cannot create a git commit message from PR title and description.
Error while parsing PR description body: the line is too long 102>72
Problematic parser input:
Note: there are some calls remaining that do not check the return value of `comm_udp_recvfrom` at all.
Please see PR title and description formatting requirements for more details.
This message was added by Anubis bot. Anubis will add a new message if the error text changes. Anubis will remove M-failed-description label when there are no corresponding failures to report.
I could not find any meaningful examples that would match the above statement well.
Here at all: https://github.com/squid-cache/squid/blob/9b6f3be2fb8d81d664dd94f08070bfe78658b67d/src/htcp.cc#L1424
Here at equaling 0: https://github.com/squid-cache/squid/blob/9b6f3be2fb8d81d664dd94f08070bfe78658b67d/src/wccp.cc#L202-L216
Let me clean these up too.