Date: Sun, 25 Feb 2001 22:59:18 +0200 (EET) From: Maxim Sobolev <sobomax@freebsd.org> To: rooneg@electricjellyfish.net (Garrett Rooney) Cc: sobomax@freebsd.org (Maxim Sobolev), ports@freebsd.org, jhk@freebsd.org, gad@freebsd.org, reg@freebsd.org Subject: Re: [patch] which package functionality for pkg_info Message-ID: <200102252059.f1PKxte31663@vic.sabbo.net> In-Reply-To: <20010225141537.A94657@electricjellyfish.net> from "Garrett Rooney" at Feb 25, 2001 02:15:38 PM
next in thread | previous in thread | raw e-mail | index | archive | help
>
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200102252059.f1PKxte31663>