From owner-cvs-src@FreeBSD.ORG Tue Mar 18 19:38:11 2008 Return-Path: Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4D55B1065672; Tue, 18 Mar 2008 19:38:11 +0000 (UTC) (envelope-from vadim@hostel.avtf.net) Received: from hostel.avtf.net (ip82-117-84-33.vpn.tomsk.net [82.117.84.33]) by mx1.freebsd.org (Postfix) with ESMTP id 52A3E8FC1E; Tue, 18 Mar 2008 19:38:08 +0000 (UTC) (envelope-from vadim@hostel.avtf.net) Received: from hostel.avtf.net (localhost [127.0.0.1]) by hostel.avtf.net (8.14.1/8.14.1) with ESMTP id m2IJKcUc038233; Wed, 19 Mar 2008 01:20:38 +0600 (NOVT) (envelope-from vadim@hostel.avtf.net) Received: (from vadim@localhost) by hostel.avtf.net (8.14.1/8.14.1/Submit) id m2IJKcoD038230; Wed, 19 Mar 2008 01:20:38 +0600 (NOVT) (envelope-from vadim) Message-Id: <200803181920.m2IJKcoD038230@hostel.avtf.net> To: Paolo Pisati From: Vadim Goncharov In-Reply-To: <200803172216.m2HMGJKj011742@repoman.freebsd.org> References: <200803172216.m2HMGJKj011742@repoman.freebsd.org> X-Comment-To: Paolo Pisati Date: Wed, 19 Mar 2008 01:20:37 +0600 Cc: cvs-src@FreeBSD.org, des@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet/libalias alias_irc.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: vadim_nuclight@mail.ru List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Mar 2008 19:38:11 -0000 Hi Paolo Pisati! On Mon, 17 Mar 2008 22:16:19 +0000 (UTC); Paolo Pisati wrote: > piso 2008-03-17 22:16:19 UTC > FreeBSD src repository > Modified files: (Branch: RELENG_7) > sys/netinet/libalias alias_irc.c > Log: > MFC revision 1.25: > Don't abuse stack space while in kernel land, use heap instead. > > PR: kern/121693 > > Revision Changes Path > 1.23.2.1 +10 -5 src/sys/netinet/libalias/alias_irc.c Paolo, this violates rules 6 and 10 from committers-guide. The original commit log in HEAD did not contained "MFC After:", so the code was not supposed to be MFCed to STABLE. Moreover, minimum MFC period is 3 days, and you've done MFCs to both 6-STABLE and 7-STABLE in 8 minutes after commit to HEAD. Then, PR must not be closed so fast, as originator has not yet confirmed that patch has fixed his problem. This could be non-critical, but that rules exists to guard against bugs in the code, so - let's look at the code. You are changing "char newpacket[65536]" to "char *newpacket", and there are places there like: if (iCopy + 4 > sizeof(newpacket)) goto lPACKET_DONE; which you hasn't changed, while "iCopy" is "unsigned int". This will break behaviour, of course, in 7-STABLE - in 6-STABLE it is still an array. It is questionable whether silent change in size from 65536 to IP_MAXPACKET (which is one less) is acceptable, but there is also another clear point: this will be allocated with M_NOWAIT on every packet. It is better to prealloc that memory on module load, the way you've done in 6-STABLE's MFC variant. -- WBR, Vadim Goncharov. ICQ#166852181 mailto:vadim_nuclight@mail.ru [Moderator of RU.ANTI-ECOLOGY][FreeBSD][http://antigreen.org][LJ:/nuclight]