I changed the code as per the suggestions. There wasn't much of a
reduction but it looks better now.
webrev: http://cr.opensolaris.org/~vassun08/mdbmacro-webrev-r7/
Thanks
Vasumathi
James Carlson wrote:
Vasumathi Sundaram writes:
Please have a look at the latest webrev at
http://cr.opensolaris.org/~vassun08/mdbmacro-webrev-r6
I would have simply allocated all of netstat_cb_data_t and embedded
conn_t within it, rather than separately allocating cbdata.conn, but
what you've done there seems fine.
Moving 'af' into netstat_cb_data_t and eliminating all those wrapper
functions looks like a significant improvement in the code. Thanks
for doing that!
As for the new netstat_print_conn and netstat_header_v[46] functions,
I don't think I understand why 'proto' is passed in as a string. It
appears to be designed as an enum, because all of the callers pass in
a fixed, hard-coded string, and all of the users of that formal
argument just do strcmp. Why not actually use either an enum or
(maybe better yet) the existing IPPROTO_* values?
It would be possible to go one further with reducing the code
duplication. You could have a function like this:
static int
netstat_print_v46(const char *cache, int proto, mdb_walk_cb_t cbfunc,
void *cbdata)
{
netstat_cb_data_t *ncb = cbdata;
int af = ncb->af;
int status = DCMD_OK;
if (af != AF_INET6) {
ncb->af = AF_INET;
status = netstat_print_conn(cache, proto, cbfunc, cbdata);
}
if (status == DCMD_OK && af != AF_INET) {
ncb->af = AF_INET6;
status = netstat_print_conn(cache, proto, cbfunc, cbdata);
}
ncb->af = af;
return (status);
}
... and then the caller would just need to do this:
cbdata.af = af;
if (optP == NULL || strcmp("tcp", optP) == 0) {
status = netstat_print_v46("tcp_conn_cache", IPPROTO_TCP,
netstat_tcp_cb, &cbdata);
if (status != DCMD_OK)
return (status);
}
if (optP == NULL || strcmp("udp", optP) == 0) {
status = netstat_print_v46("udp_conn_cache", IPPROTO_UDP,
netstat_udp_cb, &cbdata);
if (status != DCMD_OK)
return (status);
}
if (optP == NULL || strcmp("icmp", optP) == 0) {
status = netstat_print_v46("rawip_conn_cache", IPPROTO_ICMP,
netstat_icmp_cb, &cbdata);
if (status != DCMD_OK)
return (status);
}
return (DCMD_OK);
_______________________________________________
networking-discuss mailing list
[email protected]
