Date: 29 Oct 2001 15:36:38 +0100 From: Dag-Erling Smorgrav <des@ofug.org> To: "Andrew R. Reiter" <arr@watson.org> Cc: Luigi Rizzo <rizzo@aciri.org>, Josef Karthauser <joe@FreeBSD.org>, cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: ipfw.c -- (was: cvs commit: src/sys/netinet ip_fw.h) Message-ID: <xzp4roiqzrd.fsf@flood.ping.uio.no> In-Reply-To: <Pine.NEB.3.96L.1011029010151.33440A-100000@fledge.watson.org> References: <Pine.NEB.3.96L.1011029010151.33440A-100000@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
"Andrew R. Reiter" <arr@watson.org> writes:
> if (chain->fw_flg & IP_FW_F_RND_MATCH) {
> double d = 1.0 * chain->dont_match_prob;
> d = 1 - (d / 0x7fffffff);
> printf("prob %f ", d);
> }
There is no need for a temporary variable here. The probability
computation can be greatly simplified. BTW, the format string should
specify the number of decimals (e.g. "%.3f").
if (chain->fw_flg & IP_FW_F_RND_MATCH)
printf("prob %.3f ",
1.0 - chain->dont_match_prob / 0x7fffffff);
Also, the use of the hard-coded value 0x7fffffff looks suspicious. It
looks like a scaling factor, which means it's probably used in several
other places both in the kernel and in userland; it should at the very
least be a symbolic constant (e.g. PROB_SCALE_FACTOR); at best, there
should be a preprocessor macro that converts the dont_match_prob value
to a double in the appropriate range.
> if (do_time) {
> if (chain->timestamp) {
> char timestr[30];
> strcpy(timestr, ctime((time_t *)&chain->timestamp));
> *strchr(timestr, '\n') = '\0';
> printf("%s ", timestr);
> } else {
> printf(" ");
> }
> }
This code should probably use strftime() instead of ctime(), and
format the timestamp in ISO8601 format (YYYY-MM-DD HH:MM:SS). It also
makes potentially unsound assumptions about the length of the text
generated by ctime().
Both printf()s can be written more intelligently (and more obviously
correct) using width and precision specifiers.
if (do_time) {
char timestr[20];
if (chain->timestamp)
strftime(timestr, sizeof timestr, "%Y-%m-%d %H:%M:%S",
localtime(chain->timestamp));
else
timestr[0] = '\0';
printf("%-20.20s", timestr);
}
DES
--
Dag-Erling Smorgrav - des@ofug.org
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?xzp4roiqzrd.fsf>