On Mon, 09 Nov 2009 19:01:43 +0100 (CET), Alexander Best <alexbes...@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': %%%
pgp5BQPCQ7Jo2.pgp
Description: PGP signature