ping: Split JSON hostname and IP address by pevik · Pull Request #618 · iputils/iputils

pevik · 2026-04-09T12:51:26Z

Previously hostname and IP were printed together in "host" key:

{"bytes": 64, "host": "www.seznam.cz (77.75.77.222)", "seq": 1,
"ttl": 55, "time": "11.0"}
{"host": "seznam.cz", "transmitted": 1, "received": 1, "duplicates":
0, "corrupted": 0, "errors": 0, "loss": 0, "time": 0, "rtt": {"min":
"11.015", "avg": "11.015", "max": "11.015", "mdev": "0.000"}}

Now output has new array "host", with "arg", "cname" and "ip" keys, replacing the original "host" key:

{"bytes": 64, "host": {"arg": "seznam.cz", "cname": "www.seznam.cz",
"ip": "77.75.79.222"}, "seq": 1, "ttl": 55, "time": "11.4"}
{"transmitted": 1, "received": 1, "duplicates": 0, "corrupted": 0,
"errors": 0, "loss": 0, "time": 0, "rtt": {"min": "11.384", "avg":
"11.384", "max": "11.384", "mdev": "0.000"}}

arg - original argv[1] parameter (destination).
cname - getaddrinfo() reverse DNS resolution (PTR lookup), NOTE: not added if empty due calling ping with -n
ip - IPv4 or IPv6

That required rewrite handling hostname and reverse IP resolution. Previously were both hostname and IP address passed to gather_statistics() in a single string. Now function have both as separate parameters. That required adding new function set_cname_ip().

Fixes: #609

@iputils/maintainers @tacerus comments are welcome. I'm sure the implementation needs cleanup and better naming of functions and parameters.

pevik · 2026-04-09T13:33:45Z

Caching of the DNS lookup in the new function is wrong.

pevik · 2026-04-10T08:55:06Z

Caching of the DNS lookup in the new function is wrong.

At least the output of the function was fixed (previously it was even missing), but caching should be really fixed. I'll check if getaddrinfo() can be call earlier before looping so that we avoid the need to cache it. Other option is to store result in ~~struct ping_rts, which would solve caching.~~ some struct (name and addr) for a specific use for -j and add wrapper which will keep the current functionality. And keeping the same cache as previously.

pevik · 2026-04-10T10:16:36Z

Renamed cname => rdns.

pevik · 2026-04-10T11:40:02Z

It looks to me that pr_addr() wrapper which calls getaddrinfo() is mostly called for the same host (even on error, e.g. ./builddir/ping/ping -c3 -i0.01 www.seznam.cz -s 5888 which prints From www.seznam.cz (77.75.77.222) icmp_seq=3 Frag needed and DF set (mtu = 1480)), therefore I don't think we need to cache anything else than the last result (e.g. don't need to implement linked list for the cache, which would be something unrelated to the change.
@metan-ucw WDYT?

This is a preparation for a next commit which will separate IP address from hostname in JSON output. Signed-off-by: Petr Vorel <[email protected]>

Previously hostname and IP were printed together in "host" key: $ ./builddir/ping/ping -c1 -j seznam.cz {"bytes": 64, "host": "www.seznam.cz (77.75.77.222)", "seq": 1, "ttl": 55, "time": "11.0"} {"host": "seznam.cz", "transmitted": 1, "received": 1, "duplicates": 0, "corrupted": 0, "errors": 0, "loss": 0, "time": 0, "rtt": {"min": "11.015", "avg": "11.015", "max": "11.015", "mdev": "0.000"}} Now output has new array "host", with "arg", "rdns" and "ip" keys, replacing the original "host" key: {"bytes": 64, "host": {"arg": "seznam.cz", "rdns": "www.seznam.cz", "ip": "77.75.79.222"}, "seq": 1, "ttl": 55, "time": "10.5"} {"transmitted": 1, "received": 1, "duplicates": 0, "corrupted": 0, "errors": 0, "loss": 0, "time": 0, "rtt": {"min": "11.384", "avg": "11.384", "max": "11.384", "mdev": "0.000"}} * "arg" - original argv[1] parameter (destination). * "rdns" - getaddrinfo() reverse DNS resolution (PTR lookup), NOTE: not added if empty due calling ping with "-n" * "ip" - IPv4 or IPv6 That required rewrite handling hostname and reverse IP resolution. Previously were both hostname and IP address passed to gather_statistics() in a single string. Now function have both as separate parameters. That required adding new function set_rdns_ip(). TODO: Ideally construct_json_host() call would be in ping/ping_common.c, but struct pr_addr is not visible in ping/ping_common.h even ping.h is included or typedef used. Fixes: iputils#609 Signed-off-by: Petr Vorel <[email protected]>

pevik · 2026-04-27T14:13:42Z

+ *
+ * Return an ascii host address optionally with a hostname (separated).
+ */
+struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa,


Hm, it should probably be using pointer:

-struct pr_addr pr_addr_split(struct ping_rts *rts, void *sa, +struct pr_addr *pr_addr_split(struct ping_rts *rts, void *sa, socklen_t salen)

But changing it brings a core dump on certain use case:

$ r/ping/ping -c3 -i0.01 google.com -n -j ninja -C builddir ninja: Entering directory `builddir' ninja: Jobserver mode detected: -j24 --jobserver-auth=fifo:/tmp/GMfifo877643 [17/17] Linking target ping/ping Segmentation fault (core dumped) ./builddir/ping/ping -c3 -i0.01 google.com -n -j

Problem is in json_kv_str() having a garbage in json_packet->object and json_packet->size.

diff --git ping/ping.c ping/ping.c index 7167ab7..ec80d4c 100644 --- ping/ping.c +++ ping/ping.c @@ -1794,7 +1794,7 @@ int ping4_parse_reply(struct ping_rts *rts, struct socket_st *sock, * * Return an ascii host address optionally with a hostname (separated). */ -struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa, +struct pr_addr *_pr_addr_split(struct ping_rts *rts, void *sa, socklen_t salen, int resolve_name) { static struct pr_addr out = {0}; @@ -1802,7 +1802,7 @@ struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa, static socklen_t last_salen = 0; if (salen == last_salen && !memcmp(sa, &last_sa, salen)) - return out; + return &out; memcpy(&last_sa, sa, (last_salen = salen)); @@ -1814,7 +1814,7 @@ struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa, rts->in_pr_addr = 0; - return out; + return &out; } /* @@ -1822,7 +1822,7 @@ struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa, * * Return an ascii host address with a hostname (separated). */ -struct pr_addr pr_addr_split(struct ping_rts *rts, void *sa, +struct pr_addr *pr_addr_split(struct ping_rts *rts, void *sa, socklen_t salen) { return _pr_addr_split(rts, sa, salen, 1); @@ -1855,8 +1855,7 @@ char *pr_raw_addr(struct ping_rts *rts, void *sa, socklen_t salen) */ char *_pr_addr(struct ping_rts *rts, void *sa, socklen_t salen, int resolve_name) { - struct pr_addr out = _pr_addr_split(rts, sa, salen, resolve_name); - return pr_addr_format(&out); + return pr_addr_format(_pr_addr_split(rts, sa, salen, resolve_name)); } char *pr_addr_format(struct pr_addr *out) diff --git ping/ping.h ping/ping.h index a96f525..02e0403 100644 --- ping/ping.h +++ ping/ping.h @@ -422,7 +422,7 @@ struct pr_addr { }; char *pr_addr(struct ping_rts *rts, void *sa, socklen_t salen); -struct pr_addr pr_addr_split(struct ping_rts *rts, void *sa, socklen_t salen); +struct pr_addr *pr_addr_split(struct ping_rts *rts, void *sa, socklen_t salen); char *pr_raw_addr(struct ping_rts *rts, void *sa, socklen_t salen); char *pr_addr_format(struct pr_addr *out); @@ -440,7 +440,7 @@ extern void status(struct ping_rts *rts); extern void common_options(int ch); extern int gather_statistics(struct ping_rts *rts, uint8_t *icmph, int icmplen, int cc, uint16_t seq, int hops, - int csfailed, struct timeval *tv, struct pr_addr out, + int csfailed, struct timeval *tv, struct pr_addr *out, void (*pr_reply)(struct ping_rts *rts, uint8_t *ptr, int cc), int multicast, int wrong_source); extern void print_timestamp(struct ping_rts *rts); diff --git ping/ping_common.c ping/ping_common.c index 751d1ec..87ca71a 100644 --- ping/ping_common.c +++ ping/ping_common.c @@ -748,7 +748,7 @@ int main_loop(struct ping_rts *rts, ping_func_set_st *fset, socket_st *sock, int gather_statistics(struct ping_rts *rts, uint8_t *icmph, int icmplen, int cc, uint16_t seq, int hops, - int csfailed, struct timeval *tv, struct pr_addr out, + int csfailed, struct timeval *tv, struct pr_addr *out, void (*pr_reply)(struct ping_rts *rts, uint8_t *icmph, int cc), int multicast, int wrong_source) { @@ -838,9 +838,9 @@ restamp: // TODO: move into ping/ping_output.c if (rts->opt_json) - construct_json_host(rts, out.name, out.addr); + construct_json_host(rts, out->name, out->addr); else - printf(_("from %s:"), pr_addr_format(&out)); + printf(_("from %s:"), pr_addr_format(out)); if (pr_reply) pr_reply(rts, icmph, cc);

If you use pointer you have to allocate the memory somewhere. What about adding these fields to struct ping_rts?

Do you mean to add struct pr_addr to struct ping_rts? The problem is that we have kind of cache of the latest getnameinfo() result. Do you want to move this cache to struct ping_rts?

Probably the from (the destination) values should be done this way (calls in ping/ping6_common.c and ping/ping.c), but we need to keep pr_addr() available for other use because error handling in ping/ping.c uses it as well (it can get parameter of other hosts than the destination).

Also I don't even know how to trigger this error code path (it'd be great to be able to mock it).

The cache should be probably part of struct pr_addr but I haven't finished reading the code yet.

metan-ucw · 2026-04-27T14:32:57Z

+ */
+struct pr_addr _pr_addr_split(struct ping_rts *rts, void *sa,
+                           socklen_t salen, int resolve_name)
+{


It's a bit unusual to return ~2kB of data in a return value. Is there a reason not to store the data in the ping_rts instead?

pevik requested review from a team, metan-ucw and nmeyerhans April 9, 2026 12:51

pevik force-pushed the gh609 branch from 8ef46b8 to 28cedef Compare April 9, 2026 13:21

pevik force-pushed the gh609 branch from 28cedef to 7d39fb2 Compare April 9, 2026 13:44

pevik marked this pull request as draft April 9, 2026 13:45

pevik force-pushed the gh609 branch from 7d39fb2 to 73abc30 Compare April 9, 2026 14:18

pevik force-pushed the gh609 branch from 73abc30 to 5a31fae Compare April 10, 2026 10:15

pevik added 2 commits April 27, 2026 15:41

ping: Add helper function to store separated IP and hostname

3949658

This is a preparation for a next commit which will separate IP address from hostname in JSON output. Signed-off-by: Petr Vorel <[email protected]>

pevik force-pushed the gh609 branch from 5a31fae to 2e8a1e4 Compare April 27, 2026 13:53

pevik marked this pull request as ready for review April 27, 2026 13:53

pevik added the enhancement label Apr 27, 2026

pevik commented Apr 27, 2026

View reviewed changes

metan-ucw reviewed Apr 27, 2026

View reviewed changes

pevik added this to the Next release milestone Jun 2, 2026

Navigation Menu

Repository navigation

ping: Split JSON hostname and IP address - #618

ping: Split JSON hostname and IP address#618
pevik wants to merge 2 commits into
iputils:masterfrom
pevik:gh609

pevik commented Apr 9, 2026

Uh oh!

pevik commented Apr 9, 2026

Uh oh!

pevik commented Apr 10, 2026 •

edited

Loading

Uh oh!

pevik commented Apr 10, 2026

Uh oh!

pevik commented Apr 10, 2026

Uh oh!

pevik Apr 27, 2026

Uh oh!

pevik Apr 27, 2026

Uh oh!

metan-ucw Apr 27, 2026

Uh oh!

pevik Apr 27, 2026

Uh oh!

metan-ucw Apr 27, 2026

Uh oh!

metan-ucw Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

pevik commented Apr 9, 2026

Uh oh!

pevik commented Apr 9, 2026

Uh oh!

pevik commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pevik commented Apr 10, 2026

Uh oh!

pevik commented Apr 10, 2026

Uh oh!

pevik Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

pevik Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

metan-ucw Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

pevik Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

metan-ucw Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

metan-ucw Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pevik commented Apr 10, 2026 •

edited

Loading