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

lib: use int type for more port variables #6553

Closed
wants to merge 1 commit into from
Closed

Conversation

jay
Copy link
Member

@jay jay commented Feb 1, 2021

This is a follow-up to 764c6bd. Prior to that change port variables
were usually type long.

Closes #xxxx


Note that 764c6bd changed Curl_conninfo_remote to use a local port variable and not assign to conn->port... I'm not sure why that is, I think it should be conn->port shouldn't it?

@jay jay added the tidy-up label Feb 1, 2021
@jay jay requested a review from bagder February 1, 2021 07:11
@ghost
Copy link

ghost commented Feb 1, 2021

Congratulations 🎉. DeepCode analyzed your code in 26.992 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@jay
Copy link
Member Author

jay commented Feb 1, 2021

I need the question addressed before I land:

curl/lib/connect.c

Lines 693 to 703 in 796ce29

long port;
plen = sizeof(struct Curl_sockaddr_storage);
memset(&ssrem, 0, sizeof(ssrem));
if(getpeername(sockfd, (struct sockaddr*) &ssrem, &plen)) {
int error = SOCKERRNO;
failf(data, "getpeername() failed with errno %d: %s",
error, Curl_strerror(error, buffer, sizeof(buffer)));
return;
}
if(!Curl_addr2string((struct sockaddr*)&ssrem, plen,
conn->primary_ip, &port)) {

I turned that into int port but is that even necessary, shouldn't it be &conn->port (which is also int)?

@bagder
Copy link
Member

bagder commented Feb 1, 2021

conn->port should already be populated with the correct port number so this is just a way to not write it again (from 764c6bd)

@jay jay added the feature-window A merge of this requires an open feature window label Feb 2, 2021
This is a follow-up to 764c6bd. Prior to that change port variables
were usually type long.

Closes #xxxx
@jay jay closed this in cb2dc1b Feb 9, 2021
@jay jay deleted the int_port branch February 9, 2021 07:54
@jay jay removed the feature-window A merge of this requires an open feature window label Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants