[patch] burncd: honour for envar SPEED

[Giorgos Keramidas]

On Mon, 09 Nov 2009 19:01:43 +0100 (CET), Alexander Best <alexbestms at 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 <= 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=0 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
===================================================================
--- 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
===================================================================
--- usr.sbin/burncd/burncd.c	(revision 199064)
+++ usr.sbin/burncd/burncd.c	(working copy)
@@ -80,11 +80,20 @@
 	int dao = 0, eject = 0, fixate = 0, list = 0, multi = 0, preemp = 0;
 	int nogap = 0, speed = 4 * 177, test_write = 0, force = 0;
 	int block_size = 0, block_type = 0, cdopen = 0, dvdrw = 0;
-	const char *dev;
+	const char *dev, *env_speed;
 
 	if ((dev = getenv("CDROM")) == NULL)
 		dev = "/dev/acd0";
 
+	if ((env_speed = getenv("BURNCD_SPEED")) != NULL) {
+		if (strcasecmp("max", env_speed) == 0)
+			speed = CDR_MAX_SPEED;
+		else
+			speed = atoi(env_speed) * 177;
+		if (speed <= 0)
+			errx(EX_USAGE, "Invalid speed: %s", env_speed);
+	}
+
 	while ((ch = getopt(argc, argv, "def:Flmnpqs:tv")) != -1) {
 		switch (ch) {
 		case 'd':
%%%

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20091109/3531896a/attachment.pgp