Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Dec 2012 17:21:40 -0600
From:      Brooks Davis <brooks@FreeBSD.org>
To:        Mark Johnston <markjdb@gmail.com>
Cc:        Ian Lepore <freebsd@damnhippie.dyndns.org>, "src-committers@freebsd.org" <src-committers@FreeBSD.org>, d@delphij.net, Jilles Tjoelker <jilles@stack.nl>, Garrett Cooper <yanegomi@gmail.com>, "svn-src-all@freebsd.org" <svn-src-all@FreeBSD.org>, Alfred Perlstein <bright@mu.org>, Xin LI <delphij@FreeBSD.org>, "svn-src-head@freebsd.org" <svn-src-head@FreeBSD.org>, Andrey Zonov <zont@FreeBSD.org>
Subject:   Re: svn commit: r244198 - in head: etc/rc.d sbin/sysctl
Message-ID:  <20121219232140.GA40927@lor.one-eyed-alien.net>
In-Reply-To: <20121219225854.GA8399@oddish>
References:  <1355931456.1198.203.camel@revolution.hippie.lan> <05CC5BAD-B968-4A7A-8097-A3344D970D63@mu.org> <1355932607.1198.206.camel@revolution.hippie.lan> <F158CD10-B65B-4BBA-BCAD-6A52BC5C8FDF@mu.org> <50D2128A.7030205@delphij.net> <20121219210418.GA83983@stack.nl> <CAGH67wRTdBCZLq1R8-yqD6EBZ%2B=F228-0o_8TvU3b0KZD6vUfg@mail.gmail.com> <CAGH67wT1L_9ia7-hdHST6q8iCdtmFifZcHixCQhBTiZEH8YAcw@mail.gmail.com> <50D23961.7090803@delphij.net> <20121219225854.GA8399@oddish>

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

--/04w6evG8XlLl3ft
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 19, 2012 at 05:58:54PM -0500, Mark Johnston wrote:
> On Wed, Dec 19, 2012 at 02:02:09PM -0800, Xin Li wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA256
> >=20
> > On 12/19/12 13:12, Garrett Cooper wrote:
> > > On Wed, Dec 19, 2012 at 1:10 PM, Garrett Cooper
> > > <yanegomi@gmail.com> wrote:
> > >=20
> > > ...
> > >=20
> > >> find -exec / echo | xargs ? Seems like there's a better way to
> > >> solve this.
> > >=20
> > > Of course we also might be overengineering the problem (my=20
> > > suggestion definitely was overengineered). Why not pass in the=20
> > > appropriate arguments via sysctl_args in /etc/rc.conf ? Thanks,
> >=20
> > Irrelevant.  Consider this (extreme) situation: someone distributes
> > several sets of sysctl values tuned for certain situations, like
> > tcp.conf, supermicro.conf, ... and wants to put them together in a
> > directory, it's useful to source from the directory without having to
> > do a generation of command line on boot, so when something goes wrong,
> > they just remove the pack rather than changing /etc/rc.conf.
>=20
> At work I've changed the -f flag of syslogd and newsyslog to accept a
> directory which gets non-recursively searched for input files. This way
> we can have a directory, say /etc/syslog.d, into which package install
> scripts can easily drop config files.
>=20
> For something like sysctl this might be a bit much, but it's just a
> thought. The example diff below is what I have in mind.

I don't have a strong opinion about the usefulness of this feature.  It
seems useful particularly in conjunction with supporting multiple -f's.

I do have a few comments on the implementation below.

> -Mark
>=20
> diff --git a/sbin/sysctl/sysctl.c b/sbin/sysctl/sysctl.c
> index 8fad089..c880b45 100644
> --- a/sbin/sysctl/sysctl.c
> +++ b/sbin/sysctl/sysctl.c
> @@ -42,6 +42,7 @@ static const char rcsid[] =3D
>  #endif /* not lint */
> =20
>  #include <sys/param.h>
> +#include <sys/types.h>
>  #include <sys/time.h>
>  #include <sys/resource.h>
>  #include <sys/stat.h>
> @@ -49,6 +50,7 @@ static const char rcsid[] =3D
>  #include <sys/vmmeter.h>
> =20
>  #include <ctype.h>
> +#include <dirent.h>
>  #include <err.h>
>  #include <errno.h>
>  #include <inttypes.h>
> @@ -64,6 +66,7 @@ static const char *conffile;
>  static int	aflag, bflag, dflag, eflag, hflag, iflag;
>  static int	Nflag, nflag, oflag, qflag, Tflag, Wflag, xflag;
> =20
> +static int	handlefile(const char *);
>  static int	oidfmt(int *, int, char *, u_int *);
>  static int	parsefile(const char *);
>  static int	parse(const char *, int);
> @@ -165,7 +168,7 @@ main(int argc, char **argv)
> =20
>  	warncount =3D 0;
>  	if (conffile !=3D NULL)
> -		warncount +=3D parsefile(conffile);
> +		warncount +=3D handlefile(conffile);
> =20
>  	while (argc-- > 0)
>  		warncount +=3D parse(*argv++, 0);
> @@ -402,6 +405,48 @@ parse(const char *string, int lineno)
>  }
> =20
>  static int
> +handlefile(const char *filename)
> +{
> +	DIR *dir;
> +	struct dirent *de;
> +	struct stat sb;
> +	char path[MAXPATHLEN], *fname;
> +	size_t off;
> +	int warncount =3D 0;
> +
> +	if (stat(filename, &sb))
> +		err(EX_NOINPUT, "%s", filename);

I'd open the file and then use fstat here.  You will need to open it one
way or another and there's no point it checking it's type if you can't.

> +	if (S_ISREG(sb.st_mode)) {
> +		return (parsefile(filename));
> +	} else if (!S_ISDIR(sb.st_mode))
> +		errx(EX_USAGE, "invalid input file '%s'", filename);
> +
> +	dir =3D opendir(filename);

With the above suggestion, this would become just use fdopendir() here.

> +	if (dir =3D=3D NULL)
> +		err(EX_NOINPUT, "%s", filename);
> +	off =3D strlcpy(path, filename, sizeof(path) - 1);
> +	if (off >=3D sizeof(path) - 1)
> +		errx(EX_NOINPUT, "input path '%s' too long", filename);
> +
> +	fname =3D path + off;
> +	*fname++ =3D '/';
> +	off++;
> +	while ((de =3D readdir(dir)) !=3D NULL) {

Per other's suggestions I would probably only read .conf files.  A
simple fnmatch() will let you do that.

> +		strlcpy(fname, de->d_name, sizeof(path) - off);

Rather than doing string manipulation, use openat() here to open the
files in the directory.

> +		if (stat(path, &sb)) {
> +			warn("%s", path);
> +			continue;
> +		} else if (!S_ISREG(sb.st_mode))
> +			continue;
> +
> +		warncount +=3D parsefile(path);
> +	}
> +	closedir(dir);
> +
> +	return (warncount);
> +}
> +
> +static int
>  parsefile(const char *filename)

Alter to take a file descriptor instead of a path.

-- Brooks

>  {
>  	FILE *file;
>=20

--/04w6evG8XlLl3ft
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)

iD8DBQFQ0kwEXY6L6fI4GtQRAudlAKDXkrA4CnILsV0iwPhf1oz47MoligCgmoK7
RH1vSQt6VLr/Tn7N7TSi8b8=
=4OQM
-----END PGP SIGNATURE-----

--/04w6evG8XlLl3ft--



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