Date: Fri, 23 Mar 2018 20:06:33 -0700 From: Conrad Meyer <cem@freebsd.org> To: "Rodney W. Grimes" <rgrimes@freebsd.org> Cc: 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: <CAG6CVpWc43V8_uWRbUNxzGinyPgbk8UmD9rYk2dYcOLe5yPB2A@mail.gmail.com> In-Reply-To: <201803240246.w2O2kZnB033985@pdx.rh.CN85.dnsmgr.net> References: <201803232029.w2NKTYoA022545@repo.freebsd.org> <201803240246.w2O2kZnB033985@pdx.rh.CN85.dnsmgr.net>
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. 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). 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. 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpWc43V8_uWRbUNxzGinyPgbk8UmD9rYk2dYcOLe5yPB2A>