From owner-freebsd-hackers@FreeBSD.ORG Tue Feb 15 18:30:55 2011 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4B459106566B; Tue, 15 Feb 2011 18:30:55 +0000 (UTC) (envelope-from jhs@berklix.com) Received: from tower.berklix.org (tower.berklix.org [83.236.223.114]) by mx1.freebsd.org (Postfix) with ESMTP id 9445D8FC0A; Tue, 15 Feb 2011 18:30:54 +0000 (UTC) Received: from park.js.berklix.net (p5B22D05E.dip.t-dialin.net [91.34.208.94]) (authenticated bits=0) by tower.berklix.org (8.14.2/8.14.2) with ESMTP id p1FIUoYH002532; Tue, 15 Feb 2011 18:30:51 GMT (envelope-from jhs@berklix.com) Received: from fire.js.berklix.net (fire.js.berklix.net [192.168.91.41]) by park.js.berklix.net (8.13.8/8.13.8) with ESMTP id p1FIUg9e042758; Tue, 15 Feb 2011 19:30:42 +0100 (CET) (envelope-from jhs@berklix.com) Received: from fire.js.berklix.net (localhost [127.0.0.1]) by fire.js.berklix.net (8.14.3/8.14.3) with ESMTP id p1FIUWGx098299; Tue, 15 Feb 2011 19:30:37 +0100 (CET) (envelope-from jhs@fire.js.berklix.net) Message-Id: <201102151830.p1FIUWGx098299@fire.js.berklix.net> To: gljennjohn@googlemail.com From: "Julian H. Stacey" Organization: http://www.berklix.com BSD Unix Linux Consultancy, Munich Germany User-agent: EXMH on FreeBSD http://www.berklix.com/free/ X-URL: http://www.berklix.com In-reply-to: Your message "Tue, 15 Feb 2011 09:45:22 +0100." <20110215094522.383e5c47@ernst.jennejohn.org> Date: Tue, 15 Feb 2011 19:30:32 +0100 Sender: jhs@berklix.com Cc: hackers@freebsd.org, phk@freebsd.org Subject: Re: memstick.img is bloated with 7% 2K blocks of nulls 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: Tue, 15 Feb 2011 18:30:55 -0000 Gary Jennejohn wrote: > On Tue, 15 Feb 2011 01:19:45 +0100 > "Julian H. Stacey" wrote: > > > Could author phk@FreeBSD.ORG cc'd (or someone else if they are clear) > > please explain what cmdline is ? > > Possibly commit an appended // comment after "int cmdline = 0;" > > (in line 76 of 8.1 & current src/sbin/mdconfig/mdconfig.c) wgat it's for ? > > > > (cmdline seems in most places to maybe be an enum { 1 > > 2 3 } for { a d l } alternate forms of command mdconfig, > > ... & yet case 'd': sets cmdline = 3 not 2 ? > > (& 't' & 'f' sets cmdline=2 ; not 1. Puzzling. > > > > It's simply a state machine to ensure that mutually exclusive options > aren't used together. I'd wondered that, but > > ... & yet case 'd': sets cmdline = 3 not 2 ? ie both case 'd': & case 'l': cmdline = 3; which implies: Either a mistake in the code there, Or cmdline not a state table of 1,2,3 for the 3 alternate invocations 'a', 'd', 'l' shown in man mdconfig & usage(). > See the manpage; it's pretty clear from it that > only certain combinations of options are allowed. Agreed. But cmdline doesnt seem to be [just] doing that, ay least not doing case a d l = 1 2 3, else cmdline usage would be about as in my diff appended. So is cmdline is doing something else ? What ? *** mdconfig.c Tue Feb 15 18:05:56 2011 --- mdconfig_jhs.c Tue Feb 15 19:05:59 2011 *************** *** 74,79 **** --- 74,86 ---- int ch, fd, i, vflag; char *p; int cmdline = 0; + /* JHS: Assume cmdline is to indicate 1 of the 3 states shown in usage() ?, eg: + * 1 "usage: mdconfig -a -t type [-n] [-o [no]option] ... [-f file]\n" + * " [-s size] [-S sectorsize] [-u unit]\n" + * " [-x sectors/track] [-y heads/cyl]\n" + * 2 " mdconfig -d -u unit [-o [no]force]\n" + * 3 " mdconfig -l [-v] [-n] [-u unit]\n"); + */ char *mdunit; bzero(&mdio, sizeof(mdio)); *************** *** 98,104 **** usage(); action = DETACH; mdio.md_options = MD_AUTOUNIT; ! cmdline = 3; break; case 'l': if (cmdline != 0) --- 105,111 ---- usage(); action = DETACH; mdio.md_options = MD_AUTOUNIT; ! cmdline = 2; // JHS: was 3 break; case 'l': if (cmdline != 0) *************** *** 128,134 **** } else { usage(); } ! cmdline=2; break; case 'f': if (cmdline == 0) { --- 135,141 ---- } else { usage(); } ! // JHS: cmdline=2; break; case 'f': if (cmdline == 0) { *************** *** 139,147 **** /* Imply ``-t vnode'' */ mdio.md_type = MD_VNODE; mdio.md_options = MD_CLUSTER | MD_AUTOUNIT | MD_COMPRESS; ! cmdline = 2; } ! if (cmdline != 2) usage(); if (realpath(optarg, mdio.md_file) == NULL) { err(1, "could not find full path for %s", --- 146,154 ---- /* Imply ``-t vnode'' */ mdio.md_type = MD_VNODE; mdio.md_options = MD_CLUSTER | MD_AUTOUNIT | MD_COMPRESS; ! // JHS: cmdline = 2; } ! if (cmdline != 1) // JHS: was 2 usage(); if (realpath(optarg, mdio.md_file) == NULL) { err(1, "could not find full path for %s", *************** *** 200,206 **** errx(1, "Unknown option: %s.", optarg); break; case 'S': ! if (cmdline != 2) usage(); mdio.md_sectorsize = strtoul(optarg, &p, 0); break; --- 207,213 ---- errx(1, "Unknown option: %s.", optarg); break; case 'S': ! if (cmdline != 1) // JHS: Was 2 usage(); mdio.md_sectorsize = strtoul(optarg, &p, 0); break; *************** *** 214,222 **** /* Imply ``-t swap'' */ mdio.md_type = MD_SWAP; mdio.md_options = MD_CLUSTER | MD_AUTOUNIT | MD_COMPRESS; ! cmdline = 2; } ! if (cmdline != 2) usage(); mdio.md_mediasize = (off_t)strtoumax(optarg, &p, 0); if (p == NULL || *p == '\0') --- 221,229 ---- /* Imply ``-t swap'' */ mdio.md_type = MD_SWAP; mdio.md_options = MD_CLUSTER | MD_AUTOUNIT | MD_COMPRESS; ! cmdline = 1; // JHS: Was 2 } ! if (cmdline != 1) // JHS: Was 2 usage(); mdio.md_mediasize = (off_t)strtoumax(optarg, &p, 0); if (p == NULL || *p == '\0') *************** *** 236,243 **** errx(1, "Unknown suffix on -s argument"); break; case 'u': ! if (cmdline != 2 && cmdline != 3) ! usage(); if (!strncmp(optarg, "/dev/", 5)) optarg += 5; if (!strncmp(optarg, MD_NAME, sizeof(MD_NAME) - 1)) --- 243,250 ---- errx(1, "Unknown suffix on -s argument"); break; case 'u': ! // JHS: if (cmdline != 2 && cmdline != 3) ! // JHS: usage(); if (!strncmp(optarg, "/dev/", 5)) optarg += 5; if (!strncmp(optarg, MD_NAME, sizeof(MD_NAME) - 1)) *************** *** 254,265 **** vflag = OPT_VERBOSE; break; case 'x': ! if (cmdline != 2) usage(); mdio.md_fwsectors = strtoul(optarg, &p, 0); break; case 'y': ! if (cmdline != 2) usage(); mdio.md_fwheads = strtoul(optarg, &p, 0); break; --- 261,272 ---- vflag = OPT_VERBOSE; break; case 'x': ! if (cmdline != 1) // JHS: was 2 usage(); mdio.md_fwsectors = strtoul(optarg, &p, 0); break; case 'y': ! if (cmdline != 1) // JHS: was 2 usage(); mdio.md_fwheads = strtoul(optarg, &p, 0); break; *************** *** 275,285 **** fd = open("/dev/" MDCTL_NAME, O_RDWR, 0); if (fd < 0) err(1, "open(/dev/%s)", MDCTL_NAME); ! if (cmdline == 2 && (mdio.md_type == MD_MALLOC || mdio.md_type == MD_SWAP)) if (mdio.md_mediasize == 0) errx(1, "must specify -s for -t malloc or -t swap"); ! if (cmdline == 2 && mdio.md_type == MD_VNODE) if (mdio.md_file[0] == '\0') errx(1, "must specify -f for -t vnode"); if (mdio.md_type == MD_VNODE && --- 282,292 ---- fd = open("/dev/" MDCTL_NAME, O_RDWR, 0); if (fd < 0) err(1, "open(/dev/%s)", MDCTL_NAME); ! if (cmdline == 1 // JHS: was 2 && (mdio.md_type == MD_MALLOC || mdio.md_type == MD_SWAP)) if (mdio.md_mediasize == 0) errx(1, "must specify -s for -t malloc or -t swap"); ! if (cmdline == 1 && mdio.md_type == MD_VNODE) // JHS: was 2 if (mdio.md_file[0] == '\0') errx(1, "must specify -f for -t vnode"); if (mdio.md_type == MD_VNODE && *************** *** 303,309 **** return (md_query(mdunit)); } } else if (action == ATTACH) { ! if (cmdline < 2) usage(); i = ioctl(fd, MDIOCATTACH, &mdio); if (i < 0) --- 310,316 ---- return (md_query(mdunit)); } } else if (action == ATTACH) { ! if (cmdline != 1) // JHS: was < 2 usage(); i = ioctl(fd, MDIOCATTACH, &mdio); if (i < 0) Cheers, Julian -- Julian Stacey, BSD Unix Linux C Sys Eng Consultants Munich http://berklix.com Mail plain text; Not quoted-printable, Not HTML, Not base 64. Reply below text sections not at top, to avoid breaking cumulative context.