From owner-freebsd-hackers@FreeBSD.ORG Mon Nov 9 18:07:38 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7B5F51065676 for ; Mon, 9 Nov 2009 18:07:38 +0000 (UTC) (envelope-from keramida@freebsd.org) Received: from poseidon.ceid.upatras.gr (poseidon.ceid.upatras.gr [150.140.141.169]) by mx1.freebsd.org (Postfix) with ESMTP id DED698FC1C for ; Mon, 9 Nov 2009 18:07:37 +0000 (UTC) Received: from mail.ceid.upatras.gr (unknown [10.1.0.143]) by poseidon.ceid.upatras.gr (Postfix) with ESMTP id DE2EAEB47E4; Mon, 9 Nov 2009 20:07:36 +0200 (EET) Received: from localhost (europa.ceid.upatras.gr [127.0.0.1]) by mail.ceid.upatras.gr (Postfix) with ESMTP id C891F452B4; Mon, 9 Nov 2009 20:07:36 +0200 (EET) X-Virus-Scanned: amavisd-new at ceid.upatras.gr Received: from mail.ceid.upatras.gr ([127.0.0.1]) by localhost (europa.ceid.upatras.gr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id A-0bcIOzqsCT; Mon, 9 Nov 2009 20:07:36 +0200 (EET) Received: from kobe.laptop (ppp-94-64-244-12.home.otenet.gr [94.64.244.12]) by mail.ceid.upatras.gr (Postfix) with ESMTP id 60B62452A4; Mon, 9 Nov 2009 20:07:36 +0200 (EET) Received: from kobe.laptop (kobe.laptop [127.0.0.1]) by kobe.laptop (8.14.3/8.14.3) with ESMTP id nA9I7ZYo043818 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 9 Nov 2009 20:07:35 +0200 (EET) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost) by kobe.laptop (8.14.3/8.14.3/Submit) id nA9I7YFF043812; Mon, 9 Nov 2009 20:07:34 +0200 (EET) (envelope-from keramida@freebsd.org) From: Giorgos Keramidas To: Alexander Best References: Date: Mon, 09 Nov 2009 20:07:29 +0200 In-Reply-To: (Alexander Best's message of "Mon, 09 Nov 2009 19:01:43 +0100 (CET)") Message-ID: <87k4xzbmhq.fsf@kobe.laptop> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (berkeley-unix) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Cc: freebsd-hackers@freebsd.org Subject: Re: [patch] burncd: honour for envar SPEED X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Nov 2009 18:07:38 -0000 --=-=-= Content-Transfer-Encoding: quoted-printable On Mon, 09 Nov 2009 19:01:43 +0100 (CET), Alexander Best wrote: >Giorgos Keramidas schrieb am 2009-11-09: >> > i don't quite get why the value supplied with the envar has to be >> > validated. if the user supplies a speed value using the -s switch >> > no validation (except <=3D 0) is being performed either. > >> This is probably me being paranoid. I'd prefer *both* places to >> check the supplied value for invalid values, even if the check is >> something like "negative numbers are not ok". > >> > also i think there's a speed check in the atapi code. if the speed >> > requested is > the maximum driver speed it gets set to the maximum >> > driver speed automatically. > >> Your patch is fine, but as a followup commit I'd probably like seeing >> atoi() go away. AFAICT, it currently allows invalid speed values, >> defaulting to speed=3D0 when a user types: > >> burncd -s foobar [options ...] > >> We can fix that later though :) > > ok. so do you think this patch is sufficient then? once committed i'll > see if i can add some extra validation to the envar as well as the -s > switch and will also have a look at the validation the ATA code is > doing atm. Yes, the patch attached below is fine, and IMO it would be ok to commit it, minus a couple of tiny details: sorting the BURNCD_SPEED environment variable before the current CDROM item in the manpage, and bumping the manpage modification date near .Dd to today. %%% Index: usr.sbin/burncd/burncd.8 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =2D-- usr.sbin/burncd/burncd.8 (revision 199064) +++ usr.sbin/burncd/burncd.8 (working copy) @@ -164,6 +164,12 @@ .Fl f flag. .El +.Bl -tag -width ".Ev BURNCD_SPEED" +.It Ev BURNCD_SPEED +The write speed to use if one is not specified with the +.Fl s +flag. +.El .Sh FILES .Bl -tag -width ".Pa /dev/acd0" .It Pa /dev/acd0 Index: usr.sbin/burncd/burncd.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D =2D-- usr.sbin/burncd/burncd.c (revision 199064) +++ usr.sbin/burncd/burncd.c (working copy) @@ -80,11 +80,20 @@ int dao =3D 0, eject =3D 0, fixate =3D 0, list =3D 0, multi =3D 0, preemp= =3D 0; int nogap =3D 0, speed =3D 4 * 177, test_write =3D 0, force =3D 0; int block_size =3D 0, block_type =3D 0, cdopen =3D 0, dvdrw =3D 0; =2D const char *dev; + const char *dev, *env_speed; =20 if ((dev =3D getenv("CDROM")) =3D=3D NULL) dev =3D "/dev/acd0"; =20 + if ((env_speed =3D getenv("BURNCD_SPEED")) !=3D NULL) { + if (strcasecmp("max", env_speed) =3D=3D 0) + speed =3D CDR_MAX_SPEED; + else + speed =3D atoi(env_speed) * 177; + if (speed <=3D 0) + errx(EX_USAGE, "Invalid speed: %s", env_speed); + } + while ((ch =3D getopt(argc, argv, "def:Flmnpqs:tv")) !=3D -1) { switch (ch) { case 'd': %%% --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.11 (FreeBSD) iEYEARECAAYFAkr4WmYACgkQ1g+UGjGGA7biPQCbBZxm07AJZwoEWF8rNFByihOt EQQAnjtgH5Wz8erNPv1XGxFcGi9Ixerw =mZ8m -----END PGP SIGNATURE----- --=-=-=--