Date: Tue, 3 Aug 2004 13:46:40 -0400 From: Bill Moran <wmoran@potentialtech.com> To: rhempel@bmts.com Cc: freebsd-questions@freebsd.org Subject: Re: One OR MORE of source and destination addresses? Message-ID: <20040803134640.2b839e66.wmoran@potentialtech.com> In-Reply-To: <CAEBIOGHPFFJALBLJBEDCEJDGJAA.rhempel@bmts.com> References: <200408031633.I73GXIBP038908@asarian-host.net> <CAEBIOGHPFFJALBLJBEDCEJDGJAA.rhempel@bmts.com>
next in thread | previous in thread | raw e-mail | index | archive | help
"Ralph Hempel" <rhempel@bmts.com> wrote:
>
> > I just took a look at the code:
> >
> > if (q != NULL) { /* should never occur */
> > if (last_log != time_second) {
> > last_log = time_second;
> > printf("ipfw: install_state: entry already present, done\n");
> > }
> > return 0;
> > }
> >
> > What if I just hack the "printf ..." line out of there? Would that 'solve'
> > it? I know it's dirty; but would things still work?
>
> I'll jump in here as a software manager and say NO!!!!!
>
> Note, I have no idea if it will still work, but as a professional
> programmer, the question raises a number of issues :-)
>
> 1. First of all, the original programmer took time to comment
> this line:
>
> if (q != NULL) { /* should never occur */
>
> OK. There's no indication WHY it should never occur, but still, the comment
> is there.
>
> 2. By adding this line:
>
> if (last_log != time_second) {
>
> He's limiting the printed errors to one every second, so you
> are not beeing flooded with as many messages as are actually
> ocurring.
>
> Is last_log used anywhere else?
>
> 3. This line:
>
> return 0;
>
> will still return 0 if the error occurs, so the program will
> work the same with or without the diagnostic message.
>
> I'd do some more digging and find out exactly WHY this is a "should never
> occur case" to be sure that the log is not needed. If you don't print
> the log, then why do the test, except to return 0 :-)
I was thinking about this over lunch, then I saw your post ... and the
reality is that someone should really file a PR.
Mark, since you have a real-world application where this problem occurs,
it would be idea if you could file a PR with your description of what
you're trying to do and the problem it's causing.
As best I can tell, the problem is _not_ in install_state ... only the
symptom is in install_state. The problem is that code that is calling
install_state is calling it twice for some reason. Taking that into
consideration, there's a possibility that this is fixed in -CURRENT,
but I haven't found any commit entries to that tune.
--
Bill Moran
Potential Technologies
http://www.potentialtech.com
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040803134640.2b839e66.wmoran>