-
Notifications
You must be signed in to change notification settings - Fork 39
Add support for ipaddress objects in "get" method #50
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
Conversation
beac746
to
d95b236
Compare
54dd122
to
4ad1b14
Compare
Also, add a missing return when getting packed attribute fails in extension.
4ad1b14
to
c161311
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
extension/maxminddb.c
Outdated
// Although this should work on Python 3, we will hopefully delete | ||
// this soon and the Python 3 version is cleaner. | ||
const char *ipstr = PyString_AsString(obj); | ||
Py_ssize_t len = strlen(ipstr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would PyString_Size
or something be more correct? Just thinking whether it is talking about bytes or not, and whether there could be internal NULs. But I don't know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I tried it and it didn't work for some reason, but it seems to work now. I was trying to fix Python 2 support before my self-imposed deadline on Friday though so I was a bit rushed.
ipstr); | ||
return 0; | ||
} | ||
memcpy(ip_address, addresses->ai_addr, addresses->ai_addrlen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should check if addresses is null. I don't know how that would be possible though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think getaddrinfo
would have to be out of spec for that to happen, but I suppose it doesn't hurt.
extension/maxminddb.c
Outdated
|
||
static char *format_sockaddr(struct sockaddr *sa) | ||
{ | ||
char *ip = calloc(INET6_ADDRSTRLEN, sizeof(char)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check success?
extension/maxminddb.c
Outdated
addr = (char *)&sin->sin6_addr; | ||
} | ||
|
||
inet_ntop(sa->sa_family, addr, ip, INET6_ADDRSTRLEN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check success?
See also #48.