| Message ID | 1470827690-5625-1-git-send-email-phil@nwl.cc |
|---|---|
| State | Rejected, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
On 10/08/16 12:14, Phil Sutter wrote: > Instead of printing 'expires -23sec' for expired (but not yet garbage > collected) routes, print 'expired 23sec' instead. > > Signed-off-by: Phil Sutter <phil@nwl.cc> > --- > ip/iproute.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/ip/iproute.c b/ip/iproute.c > index c52294d298210..a89a26d68be0f 100644 > --- a/ip/iproute.c > +++ b/ip/iproute.c > @@ -305,6 +305,14 @@ static void print_rtax_features(FILE *fp, unsigned int features) > fprintf(fp, " 0x%x", of); > } > > +static void print_expires(FILE *fp, __s32 expires, int hz) > +{ > + if (expires > 0) > + fprintf(fp, " expires %dsec", expires/hz); > + else > + fprintf(fp, " expired %dsec", -expires/hz); > +} Perhaps something that differs by more than a single character, to make it stand out more? I don't know what to suggest, though. -Ed
On Thu, Aug 11, 2016 at 02:46:26PM +0100, Edward Cree wrote: > On 10/08/16 12:14, Phil Sutter wrote: > > Instead of printing 'expires -23sec' for expired (but not yet garbage > > collected) routes, print 'expired 23sec' instead. > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > --- > > ip/iproute.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/ip/iproute.c b/ip/iproute.c > > index c52294d298210..a89a26d68be0f 100644 > > --- a/ip/iproute.c > > +++ b/ip/iproute.c > > @@ -305,6 +305,14 @@ static void print_rtax_features(FILE *fp, unsigned int features) > > fprintf(fp, " 0x%x", of); > > } > > > > +static void print_expires(FILE *fp, __s32 expires, int hz) > > +{ > > + if (expires > 0) > > + fprintf(fp, " expires %dsec", expires/hz); > > + else > > + fprintf(fp, " expired %dsec", -expires/hz); > > +} > Perhaps something that differs by more than a single character, to > make it stand out more? I don't know what to suggest, though. Yes, I thought about that, too. At first, I had "expired for %dsec" for the second case, but decided that it was a bit too verbose given that all route info goes into a single line and is still meant to be human-readable. :) Cheers, Phil
On Thu, 11 Aug 2016 15:54:50 +0200 Phil Sutter <phil@nwl.cc> wrote: > On Thu, Aug 11, 2016 at 02:46:26PM +0100, Edward Cree wrote: > > On 10/08/16 12:14, Phil Sutter wrote: > > > Instead of printing 'expires -23sec' for expired (but not yet garbage > > > collected) routes, print 'expired 23sec' instead. > > > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > > --- > > > ip/iproute.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/ip/iproute.c b/ip/iproute.c > > > index c52294d298210..a89a26d68be0f 100644 > > > --- a/ip/iproute.c > > > +++ b/ip/iproute.c > > > @@ -305,6 +305,14 @@ static void print_rtax_features(FILE *fp, unsigned int features) > > > fprintf(fp, " 0x%x", of); > > > } > > > > > > +static void print_expires(FILE *fp, __s32 expires, int hz) > > > +{ > > > + if (expires > 0) > > > + fprintf(fp, " expires %dsec", expires/hz); > > > + else > > > + fprintf(fp, " expired %dsec", -expires/hz); > > > +} > > Perhaps something that differs by more than a single character, to > > make it stand out more? I don't know what to suggest, though. > > Yes, I thought about that, too. At first, I had "expired for %dsec" for > the second case, but decided that it was a bit too verbose given that > all route info goes into a single line and is still meant to be > human-readable. :) > > Cheers, Phil This makes sense to humans but changing the output format can break some brittle scripts that parse output of iproute tools. The risk of incompatibility is a bigger issue than the small gain in readability. Sorry, not taking this.
On Fri, Aug 12, 2016 at 12:58:15PM -0700, Stephen Hemminger wrote: > On Thu, 11 Aug 2016 15:54:50 +0200 > Phil Sutter <phil@nwl.cc> wrote: > > > On Thu, Aug 11, 2016 at 02:46:26PM +0100, Edward Cree wrote: > > > On 10/08/16 12:14, Phil Sutter wrote: > > > > Instead of printing 'expires -23sec' for expired (but not yet garbage > > > > collected) routes, print 'expired 23sec' instead. > > > > > > > > Signed-off-by: Phil Sutter <phil@nwl.cc> > > > > --- > > > > ip/iproute.c | 12 ++++++++++-- > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/ip/iproute.c b/ip/iproute.c > > > > index c52294d298210..a89a26d68be0f 100644 > > > > --- a/ip/iproute.c > > > > +++ b/ip/iproute.c > > > > @@ -305,6 +305,14 @@ static void print_rtax_features(FILE *fp, unsigned int features) > > > > fprintf(fp, " 0x%x", of); > > > > } > > > > > > > > +static void print_expires(FILE *fp, __s32 expires, int hz) > > > > +{ > > > > + if (expires > 0) > > > > + fprintf(fp, " expires %dsec", expires/hz); > > > > + else > > > > + fprintf(fp, " expired %dsec", -expires/hz); > > > > +} > > > Perhaps something that differs by more than a single character, to > > > make it stand out more? I don't know what to suggest, though. > > > > Yes, I thought about that, too. At first, I had "expired for %dsec" for > > the second case, but decided that it was a bit too verbose given that > > all route info goes into a single line and is still meant to be > > human-readable. :) > > > > Cheers, Phil > > This makes sense to humans but changing the output format can break some brittle > scripts that parse output of iproute tools. The risk of incompatibility is a > bigger issue than the small gain in readability. Sorry, not taking this. Sure, no problem. Thanks, Phil
diff --git a/ip/iproute.c b/ip/iproute.c index c52294d298210..a89a26d68be0f 100644 --- a/ip/iproute.c +++ b/ip/iproute.c @@ -305,6 +305,14 @@ static void print_rtax_features(FILE *fp, unsigned int features) fprintf(fp, " 0x%x", of); } +static void print_expires(FILE *fp, __s32 expires, int hz) +{ + if (expires > 0) + fprintf(fp, " expires %dsec", expires/hz); + else + fprintf(fp, " expired %dsec", -expires/hz); +} + int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) { FILE *fp = (FILE *)arg; @@ -502,7 +510,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) if (!hz) hz = get_user_hz(); if (ci->rta_expires != 0) - fprintf(fp, " expires %dsec", ci->rta_expires/hz); + print_expires(fp, ci->rta_expires, hz); if (ci->rta_error != 0) fprintf(fp, " error %d", ci->rta_error); if (show_stats) { @@ -530,7 +538,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) if (r->rtm_flags & RTM_F_CLONED) fprintf(fp, "%s cache ", _SL_); if (ci->rta_expires) - fprintf(fp, " expires %dsec", ci->rta_expires/hz); + print_expires(fp, ci->rta_expires, hz); if (ci->rta_error != 0) fprintf(fp, " error %d", ci->rta_error); if (show_stats) {
Instead of printing 'expires -23sec' for expired (but not yet garbage collected) routes, print 'expired 23sec' instead. Signed-off-by: Phil Sutter <phil@nwl.cc> --- ip/iproute.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)