From owner-freebsd-questions@FreeBSD.ORG Tue Aug 3 17:46:42 2004 Return-Path: Delivered-To: freebsd-questions@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EDE1716A4CF for ; Tue, 3 Aug 2004 17:46:42 +0000 (GMT) Received: from internet.potentialtech.com (h-66-167-251-6.phlapafg.covad.net [66.167.251.6]) by mx1.FreeBSD.org (Postfix) with ESMTP id 903F143D5C for ; Tue, 3 Aug 2004 17:46:42 +0000 (GMT) (envelope-from wmoran@potentialtech.com) Received: from working.potentialtech.com (pa-plum-cmts1e-68-68-113-64.pittpa.adelphia.net [68.68.113.64]) by internet.potentialtech.com (Postfix) with ESMTP id 614AC69A71; Tue, 3 Aug 2004 13:46:41 -0400 (EDT) Date: Tue, 3 Aug 2004 13:46:40 -0400 From: Bill Moran To: rhempel@bmts.com Message-Id: <20040803134640.2b839e66.wmoran@potentialtech.com> In-Reply-To: References: <200408031633.I73GXIBP038908@asarian-host.net> Organization: Potential Technologies X-Mailer: Sylpheed version 0.9.12 (GTK+ 1.2.10; i386-portbld-freebsd4.9) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit cc: freebsd-questions@freebsd.org Subject: Re: One OR MORE of source and destination addresses? X-BeenThere: freebsd-questions@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: User questions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Aug 2004 17:46:43 -0000 "Ralph Hempel" 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