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

Rewritten hostname extraction from resolver URL #164

Merged
merged 1 commit into from
Nov 19, 2023

Conversation

baranyaib90
Copy link
Contributor

Hi, this change fixes #160
Beside that I would recommend to close all the current issues. I think all of them are answered or we have nothing to do with them.

@baranyaib90
Copy link
Contributor Author

Nope, do not merge it yet 😅
I have forgot to test with IP address in the URL.

@baranyaib90
Copy link
Contributor Author

Now it is fixed, should be working properly.
Piggyback: some very minor logging fix.

uri = tmp;
static int is_ipv4_address(char *str) {
struct in6_addr addr;
return inet_pton(AF_INET, str, &addr) == 1;
Copy link
Owner

Choose a reason for hiding this comment

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

Much nicer. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hoped so. You are welcome!

return inet_pton(AF_INET, str, &addr) == 1;
}

static int hostname_from_url(const char* url_in,
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: May be worth keeping the return code comment (non-zero == success, zero = failure).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I have left a very minimal trace of that comment by indicating which value means success. I thought it is enough.
res = 1; // success

@aarond10 aarond10 merged commit 489c57e into aarond10:master Nov 19, 2023
2 checks passed
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.

Hostname is not resolved if it contains a port at the end
2 participants