From owner-svn-src-head@FreeBSD.ORG Fri Dec 21 16:58:50 2012 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6018533A; Fri, 21 Dec 2012 16:58:50 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (garage.dawidek.net [91.121.88.72]) by mx1.freebsd.org (Postfix) with ESMTP id B85F98FC0A; Fri, 21 Dec 2012 16:58:49 +0000 (UTC) Received: from localhost (58.wheelsystems.com [83.12.187.58]) by mail.dawidek.net (Postfix) with ESMTPSA id 79F8FDA6; Fri, 21 Dec 2012 17:56:33 +0100 (CET) Date: Fri, 21 Dec 2012 17:58:42 +0100 From: Pawel Jakub Dawidek To: Mark Johnston Subject: Re: svn commit: r244198 - in head: etc/rc.d sbin/sysctl Message-ID: <20121221165842.GE1397@garage.freebsd.pl> References: <1355932607.1198.206.camel@revolution.hippie.lan> <50D2128A.7030205@delphij.net> <20121219210418.GA83983@stack.nl> <50D23961.7090803@delphij.net> <20121219225854.GA8399@oddish> <20121219232140.GA40927@lor.one-eyed-alien.net> <20121219235917.GD8399@oddish> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JBi0ZxuS5uaEhkUZ" Content-Disposition: inline In-Reply-To: <20121219235917.GD8399@oddish> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) X-Mailman-Approved-At: Fri, 21 Dec 2012 17:54:40 +0000 Cc: Ian Lepore , Brooks Davis , d@delphij.net, Jilles Tjoelker , Garrett Cooper , "svn-src-all@freebsd.org" , Alfred Perlstein , "src-committers@freebsd.org" , Xin LI , "svn-src-head@freebsd.org" , Andrey Zonov X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Dec 2012 16:58:50 -0000 --JBi0ZxuS5uaEhkUZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 19, 2012 at 06:59:17PM -0500, Mark Johnston wrote: > On Wed, Dec 19, 2012 at 05:21:40PM -0600, Brooks Davis wrote: > > 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 > > > > > 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 wro= ng, > > > > 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 w= ay > > > 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. > >=20 > > I don't have a strong opinion about the usefulness of this feature. It > > seems useful particularly in conjunction with supporting multiple -f's. >=20 > I don't really either. Just thought I'd suggest it. >=20 > >=20 > > I do have a few comments on the implementation below. > >=20 >=20 > Thanks! I didn't know about openat(). Here's the regenerated diff. [...] > static int > -parsefile(const char *filename) > +handlefile(const char *filename) > +{ > + DIR *dir; > + struct dirent *de; > + struct stat sb; > + int fd, warncount =3D 0; > + > + fd =3D open(filename, O_RDONLY); > + if (fd < 0) > + err(EX_NOINPUT, "%s", filename); > + if (fstat(fd, &sb)) > + err(EX_NOINPUT, "%s", filename); > + > + if (S_ISREG(sb.st_mode)) { > + return (parsefile(fd)); > + } else if (!S_ISDIR(sb.st_mode)) You don't need { } here. > + errx(EX_USAGE, "invalid input file '%s'", filename); > + > + dir =3D fdopendir(fd); > + if (dir =3D=3D NULL) > + err(EX_NOINPUT, "%s", filename); > + while ((de =3D readdir(dir)) !=3D NULL) { > + if (fnmatch("*.conf", de->d_name, 0) !=3D 0) > + continue; > + fd =3D openat(fd, de->d_name, O_RDONLY); You override 'fd'. After first file read it will stop working. > + if (fd < 0 || fstat(fd, &sb) !=3D 0) { > + warn("%s", de->d_name); If 'fd' is >=3D 0 you should close it. > + continue; > + } else if (!S_ISREG(sb.st_mode)) > + continue; You are leaking it here as well. > + warncount +=3D parsefile(fd); > + close(fd); [...] > - fclose(file); Removing fclose() is wrong. Once you do fdopen(3) you should use fclose(), instead you removed fclose(3) and you close(2) after parsefile() returned. You leak FILE structure this way. --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl --JBi0ZxuS5uaEhkUZ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAlDUlUIACgkQForvXbEpPzRZdACfY2Jiga1LFREBbgtlJo7XlhDp t70AnjkOIcY5oj8Wboh40LEzsqZxCLCs =io2b -----END PGP SIGNATURE----- --JBi0ZxuS5uaEhkUZ--