From owner-freebsd-ports Sun Feb 25 13:16:48 2001 Delivered-To: freebsd-ports@freebsd.org Received: from isris.pair.com (isris.pair.com [209.68.2.39]) by hub.freebsd.org (Postfix) with SMTP id B71D937B4EC for ; Sun, 25 Feb 2001 13:16:41 -0800 (PST) (envelope-from rooneg@isris.pair.com) Received: (qmail 18042 invoked by uid 3130); 25 Feb 2001 21:16:33 -0000 Date: Sun, 25 Feb 2001 16:16:33 -0500 From: Garrett Rooney To: Maxim Sobolev Cc: ports@freebsd.org, jhk@freebsd.org, gad@freebsd.org, reg@freebsd.org Subject: Re: [patch] which package functionality for pkg_info Message-ID: <20010225161633.C94657@electricjellyfish.net> References: <20010225141537.A94657@electricjellyfish.net> <200102252059.f1PKxte31663@vic.sabbo.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <200102252059.f1PKxte31663@vic.sabbo.net>; from sobomax@freebsd.org on Sun, Feb 25, 2001 at 10:59:18PM +0200 Sender: owner-freebsd-ports@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.org On Sun, Feb 25, 2001 at 10:59:18PM +0200, Maxim Sobolev wrote: > > > > comments welcome as always. > > This time only cosmetics, more comprehensive review I'll try to do > tomorrow. thanks a lot! > > char *CheckPkg = NULL; > > +char *CheckFile = NULL; > > Please be carefull with spaces vs. tabs. Get an editor with space > highlighting and reviw all your patch. oops, i thought i had gotten that one... > > +/* comparison to see if two paths we're on matches the one we're looking for. */ > > Please break up long lines like the above to the several lines. done. > > +static int > > +cmp_path(const char *target, const char *current, const char *cwd) { > > Please read style(9) for the conventions used in FreeBSD code. > Particularly, function should be in the form: > > void > foo(int bar) > { > ... > } ok. > > + strncpy(temp, cwd, cwd_len); > > + strncat(temp, "/", 1); > > + strncat(temp, current, current_len); > > Use strlcpy(3) and sprlcat(3) instead of strncpy/strncat - the former > are considerably less error-prone. cool! i had never seen those before. > > + if (!strncmp(target, fixed_path, strlen(target))) > > + rval = 1; > > + else > > + rval = 0; > > Again, please make indentation consistent with the rest of code. the way it is now is a 4 space indent, with tabs used wherever there are two levels of indent, which is what the rest of the file seems to do. is there something else i should be doing? > > + FTS *ftsp; > > + FTSENT *entp; > > Please check if you can leverage yet-to-be added matchinstalled() > function (see my recent patches in the freebsd-ports list) to get names > of installed packages instead of rolling your own fts(3) loop. i'll take a look at it. > > -.Op Fl cdDfgGiIkLmopqrRsvx > > +.Op Fl cdDfgGiIkLmopqrRsvWx > > .Op Fl e Ar package > > .Op Fl l Ar prefix > > .Op Fl t Ar template > > +.Op Fl W Ar filename > > You don't have to manifest `W' twice. Please remove it from the > `cdDfgGiIkLmopqrRsvWx'. oops. thanks. -- garrett rooney Unix was not designed to stop you from rooneg@electricjellyfish.net doing stupid things, because that would http://electricjellyfish.net/ stop you from doing clever things. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-ports" in the body of the message