Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Y2038: Use time_t for commSetConnTimeout() timeout parameter #1492

Closed
wants to merge 8 commits into from
Closed

Y2038: Use time_t for commSetConnTimeout() timeout parameter #1492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
893eb8e
commSetConnTimeout: make timeout param a time_t
kinkie Sep 24, 2023
cafab42
Update src/dns_internal.cc
kinkie Sep 25, 2023
b664d1d
Convert Ipc::UdsOp::timeout to time_t
kinkie Sep 25, 2023
df9b0bf
Merge branch 'cid-1545129' of github.com:kinkie/squid into cid-1545129
kinkie Sep 25, 2023
87e6234
int->time_t in Helper::Session::requestTimeout
kinkie Sep 25, 2023
95e4ff7
Apply suggestions from code review
kinkie Sep 25, 2023
3b1e3e7
Apply suggestion from code review
kinkie Sep 25, 2023
3a2d814
fixup: Formatted changed code
rousskov Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/comm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ commUnsetFdTimeout(int fd)
}

int
commSetConnTimeout(const Comm::ConnectionPointer &conn, int timeout, AsyncCall::Pointer &callback)
commSetConnTimeout(const Comm::ConnectionPointer &conn, time_t timeout, AsyncCall::Pointer &callback)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not realize this by looking at the PR diff, but I now see two more problems related to this (merged) PR scope:

  • This function and commUnsetConnTimeout() return type should be time_t rather than int.
  • The C-style timeout cast inside this function should be removed.

@kinkie, if you can fix these problems, please do.

The latest Coverity report (defect 1547031) triggered this comment.

{
debugs(5, 3, conn << " timeout " << timeout);
assert(Comm::IsConnOpen(conn));
Expand Down
2 changes: 1 addition & 1 deletion src/comm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ void commUnsetFdTimeout(int fd);
* Set or clear the timeout for some action on an active connection.
* API to replace commSetTimeout() when a Comm::ConnectionPointer is available.
*/
int commSetConnTimeout(const Comm::ConnectionPointer &conn, int seconds, AsyncCall::Pointer &callback);
int commSetConnTimeout(const Comm::ConnectionPointer &conn, time_t seconds, AsyncCall::Pointer &callback);
int commUnsetConnTimeout(const Comm::ConnectionPointer &conn);

int ignoreErrno(int);
Expand Down
4 changes: 2 additions & 2 deletions src/dns_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,8 +833,8 @@ idnsDoSendQueryVC(nsvc *vc)
vc->busy = 1;

// Comm needs seconds but idnsCheckQueue() will check the exact timeout
const int timeout = (Config.Timeout.idns_query % 1000 ?
Config.Timeout.idns_query + 1000 : Config.Timeout.idns_query) / 1000;
const auto timeout = (Config.Timeout.idns_query % 1000 ?
Config.Timeout.idns_query + 1000 : Config.Timeout.idns_query) / 1000;
AsyncCall::Pointer nil;

commSetConnTimeout(vc->conn, timeout, nil);
Expand Down
5 changes: 3 additions & 2 deletions src/helper.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1599,8 +1599,9 @@ Helper::Session::requestTimeout(const CommTimeoutCbParams &io)
AsyncCall::Pointer timeoutCall = commCbCall(84, 4, "Helper::Session::requestTimeout",
CommTimeoutCbPtrFun(Session::requestTimeout, srv));

const int timeSpent = srv->requests.empty() ? 0 : (squid_curtime - srv->requests.front()->request.dispatch_time.tv_sec);
const int timeLeft = max(1, (static_cast<int>(srv->parent->timeout) - timeSpent));
const time_t timeSpent = srv->requests.empty() ? 0 : (squid_curtime - srv->requests.front()->request.dispatch_time.tv_sec);
const time_t minimumNewTimeout = 1; // second
const auto timeLeft = max(minimumNewTimeout, srv->parent->timeout - timeSpent);

commSetConnTimeout(io.conn, timeLeft, timeoutCall);
}
Expand Down
2 changes: 1 addition & 1 deletion src/ipc/UdsOp.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Ipc::UdsOp::conn()
return conn_;
}

void Ipc::UdsOp::setTimeout(int seconds, const char *handlerName)
void Ipc::UdsOp::setTimeout(time_t seconds, const char *handlerName)
{
typedef CommCbMemFunT<UdsOp, CommTimeoutCbParams> Dialer;
AsyncCall::Pointer handler = asyncCall(54,5, handlerName,
Expand Down
4 changes: 2 additions & 2 deletions src/ipc/UdsOp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class UdsOp: public AsyncJob
Comm::ConnectionPointer &conn(); ///< creates if needed and returns raw UDS socket descriptor

/// call timedout() if no UDS messages in a given number of seconds
void setTimeout(int seconds, const char *handlerName);
void setTimeout(time_t seconds, const char *handlerName);
void clearTimeout(); ///< remove previously set timeout, if any

void setOptions(int newOptions); ///< changes socket options
Expand Down Expand Up @@ -92,7 +92,7 @@ class UdsSender: public UdsOp
private:
TypedMsgHdr message; ///< what to send
int retries; ///< how many times to try after a write error
int timeout; ///< total time to send the message
time_t timeout; ///< total time to send the message
bool sleeping; ///< whether we are waiting to retry a failed write
bool writing; ///< whether Comm started and did not finish writing

Expand Down
2 changes: 1 addition & 1 deletion src/tests/stub_comm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ int comm_udp_sendto(int, const Ip::Address &, const void *, int) STUB_RETVAL(-1)
void commCallCloseHandlers(int) STUB
void commUnsetFdTimeout(int) STUB
// int commSetTimeout(const Comm::ConnectionPointer &, int, AsyncCall::Pointer&) STUB_RETVAL(-1)
int commSetConnTimeout(const Comm::ConnectionPointer &, int, AsyncCall::Pointer &) STUB_RETVAL(-1)
int commSetConnTimeout(const Comm::ConnectionPointer &, time_t, AsyncCall::Pointer &) STUB_RETVAL(-1)
int commUnsetConnTimeout(const Comm::ConnectionPointer &) STUB_RETVAL(-1)
int ignoreErrno(int) STUB_RETVAL(-1)
void commCloseAllSockets(void) STUB
Expand Down