Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 16 Jan 2012 18:56:27 +0100
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Guy Helmer <guy.helmer@palisadesystems.com>
Cc:        svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Bruce Evans <brde@optusnet.com.au>
Subject:   Re: svn commit: r230037 - head/lib/libutil
Message-ID:  <20120116175627.GA1674@garage.freebsd.pl>
In-Reply-To: <52A73054-9960-403B-B2FE-857C8801D129@palisadesystems.com>
References:  <201201122249.q0CMnaZe030200@svn.freebsd.org> <20120114204720.Q1458@besplex.bde.org> <20120114182758.GJ1694@garage.freebsd.pl> <20120115073823.O843@besplex.bde.org> <52A73054-9960-403B-B2FE-857C8801D129@palisadesystems.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--5mCyUwZo2JvN/JJP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 16, 2012 at 11:14:32AM -0600, Guy Helmer wrote:
> I've pasted the diff below that I think captures the majority of the issu=
es you have brought up. I have not attempted to tackle the property.3/prope=
rties.3 issues, nor the objections to the prefixes that I think would take =
considerably more effort to resolve -- I wanted to concentrate on the issue=
s that can be isolated to libutil. I hope the diff was pasted OK, especiall=
y WRT <tab> characters.

The patch looks mostly good, one nit mentioned below and also one
question for Bruce.

> +/* Flags for hexdump(3) */
> +/* Flags for humanize_number(3) flags */
> +/* Flags for humanize_number(3) scale */
> +/* return values from realhostname() */
> +/* Flags for pw_scan() */
> +/* Return values from uu_lock() */

All those sentences are missing period and one doesn't start with
capital letter.

I noticed also one more inconsistency:

struct kinfo_file *
	kinfo_getfile(pid_t _pid, int *_cntp);
struct passwd
	*pw_dup(const struct passwd *_pw);

Sometimes * is on the same line as function type and sometimes it is in
the line below. Former is definiately better.

Guy, feel free to commit what you got now with those sentences fixed and
I'll do one iterration. It is taking way too long and I'm sure you are
bored by now:) We don't want to scare you off:)

> +struct pidfh *
> +	pidfile_open(const char *_path, mode_t _mode, pid_t *_pidptr);

Bruce, is this your suggestion? This somehow looks weird too me. What I
use and I think it is in general more widely used across FreeBSD is
simply:

struct pidfh *pidfile_open(const char *_path, mode_t _mode, pid_t *_pidptr);

when the type exceeds one tab, but the line fits into 80 chars or:

struct pidfh *pidfile_open(const char *_path, mode_t _mode, pid_t *_pidptr,
	    int _some_other_argument);

when line exceeds 80 chars.

Especially this one looks very strange:

properties
	properties_read(int _fd);

--=20
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl

--5mCyUwZo2JvN/JJP
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (FreeBSD)

iEYEARECAAYFAk8UZMsACgkQForvXbEpPzTHSQCdHx1e9M+ukC+AYW8a6r/W5OGy
IyYAn1JFpituzhEjBcHsLSD5z+DgCZzC
=ZRKx
-----END PGP SIGNATURE-----

--5mCyUwZo2JvN/JJP--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120116175627.GA1674>