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

changed int to uint16_t for port variables #1436

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WHYHD
Copy link

@WHYHD WHYHD commented Feb 21, 2024

Approach:

  • Search for variables named port or similar (i.e. _port)
  • Swap int with uint16_t
  • If this effected a method signature search for calls and add uint16_t type casting

This is same scale example to gain feedback on changing ports to unint16_t. I would like to make these changes everywhere so any feedback is welcome.

… for redisLibeventAttach method, and added casting to uint16_t when redisLibeventAttach method is called
Copy link
Collaborator

@eakraly eakraly left a comment

Choose a reason for hiding this comment

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

Left one comment to validate

@@ -268,7 +268,7 @@ redis_context_handle get_redis_async_connection(struct event_base *base, redis_s
}
}

ret = redisLibeventAttach(base, co->host, co->port, co->password, atoi(co->dbname));
ret = redisLibeventAttach(base, co->host, (uint16_t)co->port, co->password, atoi(co->dbname));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls review if you can change the type to uint16_t in the struct rather than adding cast

@eakraly
Copy link
Collaborator

eakraly commented Feb 21, 2024

Thank you @WHYHD for the contribution
The only comment would be to try and get the types right without casting - will be hard to get rid of them later

@@ -268,7 +268,7 @@
}
}

ret = redisLibeventAttach(base, co->host, co->port, co->password, atoi(co->dbname));
ret = redisLibeventAttach(base, co->host, (uint16_t)co->port, co->password, atoi(co->dbname));

Check warning

Code scanning / PREfast

'co->dbname' could be '0'.

'co->dbname' could be '0'.
@@ -134,9 +134,9 @@ static Ryconninfo *RyconninfoParse(const char *userdb, char **errmsg) {
} else if (!strcmp(s, "secret")) {
co->password = strdup(seq + 1);
} else if (!strcmp(s, "port")) {
co->port = (unsigned int)atoi(seq + 1);
co->port = atoi(seq + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you need a cast to uint16_t to avoid the compiler warning about precision loss?

@@ -210,7 +210,7 @@ redis_context_handle get_redis_async_connection(struct event_base *base, redis_s
}

if (co->port) {
port = (int)(co->port);
port = (co->port);
Copy link
Contributor

Choose a reason for hiding this comment

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

parenthesis aren't needed here anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants