From owner-freebsd-hackers Wed Mar 22 23:39:15 1995 Return-Path: hackers-owner Received: (from majordom@localhost) by freefall.cdrom.com (8.6.10/8.6.6) id XAA03030 for hackers-outgoing; Wed, 22 Mar 1995 23:39:15 -0800 Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.34]) by freefall.cdrom.com (8.6.10/8.6.6) with ESMTP id XAA03012 for ; Wed, 22 Mar 1995 23:38:57 -0800 Received: (from bde@localhost) by godzilla.zeta.org.au (8.6.9/8.6.9) id RAA05683; Thu, 23 Mar 1995 17:35:35 +1000 Date: Thu, 23 Mar 1995 17:35:35 +1000 From: Bruce Evans Message-Id: <199503230735.RAA05683@godzilla.zeta.org.au> To: PVinci@ix.netcom.com, hackers@FreeBSD.org Subject: Re: suggestions/questions for WD.C Sender: hackers-owner@FreeBSD.org Precedence: bulk >in wd.c, > static int wdcommand(struct disk *du, u_int clyinder, u_int > head, u_int sector, u_int count, u_int command); >Can't this be reduced to: > static int wdcommand(struct disk *du, u_int command); >because blknum is derived from du and cyl,head,sector are translated >before each call. Wouldn't this be cleaner? Yes. John fixed the multiple translation a couple of days ago, so the only wastage no is passing all those parameters around, but it's still wastage. The translation used to be cleaner if done in wdstart() (in the source code if not in space/time) because the C/H/S values were required for comparision with the raw BAD144 data. Several things would become less clean: the wdcommand() in wdsetctlr() would have to combine the parameters; so would new (misplaced) wdcommand()s in wdgetctlr() to set multi-mode and features; and the `#if 0' code in wddump() to print the C/H/S values would become mouldier. >in wdcommand: >if LBAmode { > LBA Translation } >else > { > CHS Translation } >This seems to be much cleaner to me... It's would be messier if you handled all the special commands. E.g., wdsetctlr() would have to check `LBAmode' to decide how to combine the paramters. wdcommand() should probably be split up. It's like it is because I got annoyed at sloppy and inconsistent timeout and error handling for different commands. >(I'd be glad to submit the changes, if someone would tell me how to >submit them via e-mail or ftp. I assume I DL WD.C in -current --make >changes and UL modified WD.C to WHO??) Small changes can be mailed the current or hackers mailing list. Lots of people are hacking on wd.c now so don't expect any particular changes to be used. >also, repeatedly, int wdc is locally defined over and over again as >du->dk_port. Isn't it nore efficient to replace this with a >#define WDC du->dk_port?? In my scanning the code WDC is really a >constant, so isn't it better to resolve it at compile time? It isn't constant. `wdc = du->dk_port' is better than du->dk_port iff the compiler can keep wdc in a register. `#define WDC(du) ((du)->dk_port)' is worse for efficiency because it hides the indirection -- unless the compiler can tell that du->dk_port can be kept in a register...hmm, gcc seems to do the right thing for `const' struct members. The FreeBSD sources currently aren't const-poisoned enough for gcc to optimize well :-). Bruce