Date: Mon, 09 Nov 2009 20:07:29 +0200 From: Giorgos Keramidas <keramida@freebsd.org> To: Alexander Best <alexbestms@wwu.de> Cc: freebsd-hackers@freebsd.org Subject: Re: [patch] burncd: honour for envar SPEED Message-ID: <87k4xzbmhq.fsf@kobe.laptop> In-Reply-To: <permail-20091109180143f0889e84000046bf-a_best01@message-id.uni-muenster.de> (Alexander Best's message of "Mon, 09 Nov 2009 19:01:43 %2B0100 (CET)") References: <permail-20091109180143f0889e84000046bf-a_best01@message-id.uni-muenster.de>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
Content-Transfer-Encoding: quoted-printable
On Mon, 09 Nov 2009 19:01:43 +0100 (CET), Alexander Best <alexbestms@wwu.de=
> 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-----
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?87k4xzbmhq.fsf>