Re: svn commit: r364321 - head/sbin/ipfw
On Mon, 31 Aug 2020 at 10:09, Rodney W. Grimes
wrote:
>
> > Hrm, it seems this reply ended up in my spam folder; sorry for not
> > replying until now.
>
> lol Oh, bad filter :-)
>
> > > > *strchr(timestr, '\n') = '\0';
> > > > bprintf(bp, "%s ", timestr);
> > >^ Isnt this the +1 space?
> > >
> > > > } else {
> > > > - bprintf(bp, "%*s", twidth, " ");
> > > > + bprintf(bp, "%*s", twidth + 1, " ");
> > > ^missing from this string?
> >
> > Inserting an extra space in the format string would also work, sure. I
> > considered doing it that way but in the end decided it's not
> > materially more clear one way or another, so used the patch as
> > submitted.
>
> For me the + 1 leads to a "why is this here", where as the space
> in the format string clearly matches the other condition of the else.
> Also + 1 causes a run time computation, the extra space does not.
No, but the extra space adds a format string character which will be
more costly than the +1. I would have used "%*s " if twidth was
already used in the other case, to keep them consistent.
Anyhow, I think this isn't worth bikeshedding; I have no objection if
anyone feels "%*s " is more clear and wants to commit that change.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364321 - head/sbin/ipfw
Am 31.08.20 um 16:09 schrieb Rodney W. Grimes:
Hrm, it seems this reply ended up in my spam folder; sorry for not
replying until now.
lol Oh, bad filter :-)
*strchr(timestr, '\n') = '\0';
bprintf(bp, "%s ", timestr);
^ Isnt this the +1 space?
} else {
- bprintf(bp, "%*s", twidth, " ");
+ bprintf(bp, "%*s", twidth + 1, " ");
^missing from this string?
Inserting an extra space in the format string would also work, sure. I
considered doing it that way but in the end decided it's not
materially more clear one way or another, so used the patch as
submitted.
For me the + 1 leads to a "why is this here", where as the space
in the format string clearly matches the other condition of the else.
In this case it doesn't seem to make much of a difference, since
the string passed is just a blank character (and it does not make
any difference whether a blank is inserted before or behind it).
But in general, I'd rather see "%*s " as the format (which makes
it clear, that the blank is inserted behind the string argument
referenced by %s, not to the left of it - or " %*s" for the other
placement of the extra blank).
But in this particular case I'd use
printf(bp, "%*s ", twidth, "");
since there is no need to pass a non-zero length argument for %s and
I'd think that the purpose of the format string is to make the columns
line up and then to provide one additional blank as a separator for
maximum length values.
Adding 1 to the width argument of "%*s" adds blanks to the left of
the string, which is obvious given the semantics of the format string,
but still takes some thinking compared to trivially seeing the place
where the extra blank is put in a format string ...
Also + 1 causes a run time computation, the extra space does not.
Well, the parsing of the extra blank in the format string can be
assumed to cost more cycles than the increment of a variable plus
the extra loop iteration it causes for the alignment blanks that are
written. But the code length will be shorter with the blank in the
format string and the extra cycles will be irrelevant ...
Regards, STefan
OpenPGP_signature
Description: OpenPGP digital signature
Re: svn commit: r364321 - head/sbin/ipfw
> Hrm, it seems this reply ended up in my spam folder; sorry for not
> replying until now.
lol Oh, bad filter :-)
> > > *strchr(timestr, '\n') = '\0';
> > > bprintf(bp, "%s ", timestr);
> >^ Isnt this the +1 space?
> >
> > > } else {
> > > - bprintf(bp, "%*s", twidth, " ");
> > > + bprintf(bp, "%*s", twidth + 1, " ");
> > ^missing from this string?
>
> Inserting an extra space in the format string would also work, sure. I
> considered doing it that way but in the end decided it's not
> materially more clear one way or another, so used the patch as
> submitted.
For me the + 1 leads to a "why is this here", where as the space
in the format string clearly matches the other condition of the else.
Also + 1 causes a run time computation, the extra space does not.
--
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364321 - head/sbin/ipfw
Hrm, it seems this reply ended up in my spam folder; sorry for not
replying until now.
> > *strchr(timestr, '\n') = '\0';
> > bprintf(bp, "%s ", timestr);
>^ Isnt this the +1 space?
>
> > } else {
> > - bprintf(bp, "%*s", twidth, " ");
> > + bprintf(bp, "%*s", twidth + 1, " ");
> ^missing from this string?
Inserting an extra space in the format string would also work, sure. I
considered doing it that way but in the end decided it's not
materially more clear one way or another, so used the patch as
submitted.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364321 - head/sbin/ipfw
[ Charset UTF-8 unsupported, converting... ]
> Author: emaste
> Date: Mon Aug 17 18:53:23 2020
> New Revision: 364321
> URL: https://svnweb.freebsd.org/changeset/base/364321
>
> Log:
> ipfw: line up `ipfw -t list` with and without timestamp
>
> From the PR:
> When I run `ipfw -t list` on release/12 or current, I get misaligned
> output between lines that do and do not have a last match timestamp,
> like so:
>
> 00100 Tue Aug 11 03:03:26 2020 allow ip from any to any via lo0
> 00200 deny ip from any to 127.0.0.0/8
>
> (specifically, the "allow" and "deny" strings do not line up)
>
> PR: 248608
> Submitted by: Taylor Stearns
> MFC after: 3 days
>
> Modified:
> head/sbin/ipfw/ipfw2.c
>
> Modified: head/sbin/ipfw/ipfw2.c
> ==
> --- head/sbin/ipfw/ipfw2.cMon Aug 17 17:48:28 2020(r364320)
> +++ head/sbin/ipfw/ipfw2.cMon Aug 17 18:53:23 2020(r364321)
> @@ -2199,7 +2199,7 @@ show_static_rule(struct cmdline_opts *co, struct forma
> *strchr(timestr, '\n') = '\0';
> bprintf(bp, "%s ", timestr);
^ Isnt this the +1 space?
> } else {
> - bprintf(bp, "%*s", twidth, " ");
> + bprintf(bp, "%*s", twidth + 1, " ");
^missing from this string?
> }
> }
>
>
--
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
