Date: Fri, 23 Mar 2018 21:35:11 -0700 (PDT) From: "Rodney W. Grimes" <freebsd@pdx.rh.CN85.dnsmgr.net> To: cem@freebsd.org Cc: "Rodney W. Grimes" <rgrimes@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-user@freebsd.org Subject: Re: svn commit: r331461 - in user/markj/netdump/sys: kern netinet/netdump sys vm Message-ID: <201803240435.w2O4ZBIn034490@pdx.rh.CN85.dnsmgr.net> In-Reply-To: <CAG6CVpWc43V8_uWRbUNxzGinyPgbk8UmD9rYk2dYcOLe5yPB2A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> The code review link is https://reviewboard.west.isilon.com/r/67553/ . > It won't do you much good outside our corporate network, though. Then it should probably not be mentioned in a FreeBSD svn commit? > Here's a summary of the my feedback, which are pretty straightforward > style-y cleanups and could easily be read out of the diff itself: > > 1. Move external function declarations from two .c files to a header > (sys/mbuf.h). > 2. Annotate unused parameters with __unused. > 3. Replace unused M_FOO flags with 0 in mb_ctor_clust() invocation. > Since valid M_FOO values are non-zero, this should result in an > assertion if the APIs suddenly start using the passed flag value. > 4. Add an assertion that mutually exclusive flags to zone_ctor > (NOBUCKET | MAXBUCKET) are indeed mutually exclusive (NOBUCKET is new > to this branch). Thank you > The only change that isn't related to my feedback was: Drop trashing > netdump mbufs during zone import subroutine. > > There are some further review comments which may eventually end up in > this branch. The intent is to merge this branch to the main line at some point, so it is of benifit to have the commit messages here hold the same quality as if it was a commit to -current, correct? > Conrad > > On Fri, Mar 23, 2018 at 7:46 PM, Rodney W. Grimes > <freebsd@pdx.rh.cn85.dnsmgr.net> wrote: > >> Author: markj > >> Date: Fri Mar 23 20:29:34 2018 > >> New Revision: 331461 > >> URL: https://svnweb.freebsd.org/changeset/base/331461 > >> > >> Log: > >> Address some but not all review feedback from cem. > > > > Please be detailed in what is changing, as the above > > only tells us where it came from in a vague way as > > no review number is even cited. > > > > I know this is on a private branch, but when/if it > > is merged this becomes part of the main line. > -- Rod Grimes rgrimes@freebsd.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201803240435.w2O4ZBIn034490>