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 > > > > On 12/19/12 13:12, Garrett Cooper wrote: > > > On Wed, Dec 19, 2012 at 1:10 PM, Garrett Cooper > > > <yaneg...@gmail.com> wrote: > > > > > > ... > > > > > >> find -exec / echo | xargs ? Seems like there's a better way to > > >> solve this. > > > > > > Of course we also might be overengineering the problem (my > > > suggestion definitely was overengineered). Why not pass in the > > > appropriate arguments via sysctl_args in /etc/rc.conf ? Thanks, > > > > 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. > > 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. > > 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
>
> 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[] =
> #endif /* not lint */
>
> #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[] =
> #include <sys/vmmeter.h>
>
> #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;
>
> +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)
>
> warncount = 0;
> if (conffile != NULL)
> - warncount += parsefile(conffile);
> + warncount += handlefile(conffile);
>
> while (argc-- > 0)
> warncount += parse(*argv++, 0);
> @@ -402,6 +405,48 @@ parse(const char *string, int lineno)
> }
>
> static int
> +handlefile(const char *filename)
> +{
> + DIR *dir;
> + struct dirent *de;
> + struct stat sb;
> + char path[MAXPATHLEN], *fname;
> + size_t off;
> + int warncount = 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 = opendir(filename);
With the above suggestion, this would become just use fdopendir() here.
> + if (dir == NULL)
> + err(EX_NOINPUT, "%s", filename);
> + off = strlcpy(path, filename, sizeof(path) - 1);
> + if (off >= sizeof(path) - 1)
> + errx(EX_NOINPUT, "input path '%s' too long", filename);
> +
> + fname = path + off;
> + *fname++ = '/';
> + off++;
> + while ((de = readdir(dir)) != 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 += 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;
>
pgpZ2njp3FhrX.pgp
Description: PGP signature
