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.

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

dnsdist: Fix several signed/unsigned comparison warnings on ARM #5597

Merged
merged 2 commits into from Sep 14, 2017

Conversation

@pieterlexis
Copy link
Member

@pieterlexis pieterlexis commented Aug 11, 2017

Short description

Closes #5489

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
This would lead to signed/unsigned comparison warnings on ARM.
@@ -190,7 +190,7 @@ static bool sendResponseToClient(int fd, const char* response, uint16_t response
static bool maxConnectionDurationReached(unsigned int maxConnectionDuration, time_t start, unsigned int& remainingTime)
{
if (maxConnectionDuration) {
time_t elapsed = time(NULL) - start;
unsigned int elapsed = time(NULL) - start;

This comment has been minimized.

@rgacogne

rgacogne Aug 11, 2017
Member

It probably doesn't matter much but this would lead to closing the connection right away if the time goes backward, because then elapsed becomes huge instead of negative. Perhaps we should store time(nullptr), and if it's lesser than start set elapsed to 0.

@pieterlexis pieterlexis force-pushed the pieterlexis:dnsdist-signed-unsigned-arm branch from 2dbfb92 to 0b75a27 Aug 14, 2017
@Habbie Habbie modified the milestones: dnsdist-1.2.0, dnsdist-1.3.0 Aug 16, 2017
@pieterlexis pieterlexis merged commit 6031f8d into PowerDNS:master Sep 14, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pieterlexis pieterlexis deleted the pieterlexis:dnsdist-signed-unsigned-arm branch Sep 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.