You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by enixon <gi...@git.apache.org> on 2018/06/22 18:39:18 UTC
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
GitHub user enixon opened a pull request:
https://github.com/apache/zookeeper/pull/547
ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/enixon/zookeeper ZOOKEEPER-3068
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/547.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #547
----
commit cd4045342ff14dd008bff95d1f54f925d8603bff
Author: Brian Nixon <ni...@...>
Date: 2018-06-22T18:23:37Z
ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
----
---
[GitHub] zookeeper issue #547: ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on the issue:
https://github.com/apache/zookeeper/pull/547
Tests timed out, probably Jenkins server were busy (and tests are flaky and/or integration tests rather then unit tests).
Please re-run the jenkins build (an empty git commit --amend will do)
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/547
---
[GitHub] zookeeper issue #547: ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/547
Committed to master branch. Thanks @enixon !
---
[GitHub] zookeeper issue #547: ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
Posted by enixon <gi...@git.apache.org>.
Github user enixon commented on the issue:
https://github.com/apache/zookeeper/pull/547
Fixed the merge conflict - this is gtg
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by enixon <gi...@git.apache.org>.
Github user enixon commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/547#discussion_r200194341
--- Diff: src/c/src/zookeeper.c ---
@@ -4358,7 +4358,9 @@ static const char* format_endpoint_info(const struct sockaddr_storage* ep)
{
static char buf[128] = { 0 };
char addrstr[128] = { 0 };
+ const char *fmtstring;
void *inaddr;
+ char is_inet6 = 0; // poor man's boolean
--- End diff --
Continuity of style.
There are no usages of `bool` in the c client yet, probably a legacy of the time it was written. Usually an `int` is used (see `disable_reconnection_attempt` in zk_adaptor.h) which is why I copied that convention in my first pass. Other times a `char` is used (see `allow_read_only` in zk_adaptor.h) and as that is slightly more efficient, I was happy to change to it.
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by enixon <gi...@git.apache.org>.
Github user enixon commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/547#discussion_r197925813
--- Diff: src/c/src/zookeeper.c ---
@@ -4358,7 +4358,9 @@ static const char* format_endpoint_info(const struct sockaddr_storage* ep)
{
static char buf[128] = { 0 };
char addrstr[128] = { 0 };
+ const char *fmtstring;
void *inaddr;
+ int inet6 = 0; // poor man's boolean
--- End diff --
Sounds good to me, I'll rework that variable.
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/547#discussion_r200195856
--- Diff: src/c/src/zookeeper.c ---
@@ -4358,7 +4358,9 @@ static const char* format_endpoint_info(const struct sockaddr_storage* ep)
{
static char buf[128] = { 0 };
char addrstr[128] = { 0 };
+ const char *fmtstring;
void *inaddr;
+ char is_inet6 = 0; // poor man's boolean
--- End diff --
Thanks, that's fine. I think we can live with it.
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/547#discussion_r197724069
--- Diff: src/c/src/zookeeper.c ---
@@ -4358,7 +4358,9 @@ static const char* format_endpoint_info(const struct sockaddr_storage* ep)
{
static char buf[128] = { 0 };
char addrstr[128] = { 0 };
+ const char *fmtstring;
void *inaddr;
+ int inet6 = 0; // poor man's boolean
--- End diff --
Just a nitpick for future readability - Maybe just java speaking from me, but why not use some more clear name like is_inet6?
(Or include stdbool)
---
[GitHub] zookeeper issue #547: ZOOKEEPER-3068: Improve C client logging of IPv6 hosts
Posted by enixon <gi...@git.apache.org>.
Github user enixon commented on the issue:
https://github.com/apache/zookeeper/pull/547
Two java tests failed - the pull request contains no java changes.
---
[GitHub] zookeeper pull request #547: ZOOKEEPER-3068: Improve C client logging of IPv...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/547#discussion_r200127778
--- Diff: src/c/src/zookeeper.c ---
@@ -4358,7 +4358,9 @@ static const char* format_endpoint_info(const struct sockaddr_storage* ep)
{
static char buf[128] = { 0 };
char addrstr[128] = { 0 };
+ const char *fmtstring;
void *inaddr;
+ char is_inet6 = 0; // poor man's boolean
--- End diff --
Why not using `bool`?
---