From owner-freebsd-ports  Sun Feb 25 13: 0: 0 2001
Delivered-To: freebsd-ports@freebsd.org
Received: from blizzard.sabbo.net (pop3.sabbo.net [193.193.218.18])
	by hub.freebsd.org (Postfix) with ESMTP
	id 0A09F37B491; Sun, 25 Feb 2001 12:59:51 -0800 (PST)
	(envelope-from max@vic.sabbo.net)
Received: from vic.sabbo.net (root@vic.sabbo.net [193.193.218.112])
	by blizzard.sabbo.net (8.10.1/8.10.1) with ESMTP id f1PKxiv13211;
	Sun, 25 Feb 2001 22:59:45 +0200
Received: (from max@localhost)
	by vic.sabbo.net (8.11.2/8.11.2) id f1PKxte31663;
	Sun, 25 Feb 2001 22:59:55 +0200 (EET)
	(envelope-from sobomax@FreeBSD.org)
From: Maxim Sobolev <sobomax@freebsd.org>
Message-Id: <200102252059.f1PKxte31663@vic.sabbo.net>
Subject: Re: [patch] which package functionality for pkg_info
To: rooneg@electricjellyfish.net (Garrett Rooney)
Date: Sun, 25 Feb 2001 22:59:18 +0200 (EET)
Cc: sobomax@freebsd.org (Maxim Sobolev), ports@freebsd.org,
	jhk@freebsd.org, gad@freebsd.org, reg@freebsd.org
In-Reply-To: <20010225141537.A94657@electricjellyfish.net> from "Garrett Rooney" at Feb 25, 2001 02:15:38 PM
X-Mailer: ELM [version 2.5 PL3]
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: owner-freebsd-ports@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

> 
> comments welcome as always.

This time only cosmetics, more comprehensive review I'll try to do
tomorrow.

>  char *CheckPkg		= NULL;
> +char *CheckFile         = NULL;

Please be carefull with spaces vs. tabs. Get an editor with space
highlighting and reviw all your patch.

> +/* 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.

> +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)
{
...
}

> +    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.

> +    if (!strncmp(target, fixed_path, strlen(target)))
> +	rval = 1;
> +    else
> +	rval = 0;

Again, please make indentation consistent with the rest of code.

> +    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.

> +	free(buf);
> +    } else
> +	res_file = file;

Identation, see above.

> -.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'.

-Maxim

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ports" in the body of the message