From owner-svn-src-all@freebsd.org Thu Aug 16 06:59:28 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 02BDD107F7B5; Thu, 16 Aug 2018 06:59:28 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 90CF670F67; Thu, 16 Aug 2018 06:59:27 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro-2.local (unknown [51.52.172.98]) by mail.baldwin.cx (Postfix) with ESMTPSA id 9E32010A87D; Thu, 16 Aug 2018 02:59:25 -0400 (EDT) Subject: Re: svn commit: r337878 - head/stand/i386/libi386 To: Warner Losh , Ian Lepore References: <201808152225.w7FMP5J2018006@repo.freebsd.org> <1534372134.1466.21.camel@freebsd.org> Cc: Toomas Soome , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org From: John Baldwin Message-ID: <05b36083-9f4d-658e-ab39-e8c317d01db7@FreeBSD.org> Date: Thu, 16 Aug 2018 07:59:23 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Thu, 16 Aug 2018 02:59:26 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Aug 2018 06:59:28 -0000 On 8/15/18 11:59 PM, Warner Losh wrote: > > > On Wed, Aug 15, 2018 at 4:28 PM, Ian Lepore > wrote: > > On Wed, 2018-08-15 at 22:25 +0000, Toomas Soome wrote: > > Author: tsoome > > Date: Wed Aug 15 22:25:05 2018 > > New Revision: 337878 > > URL: https://svnweb.freebsd.org/changeset/base/337878 > > > > Log: > >   libi386: remove bd_read() and bd_write() wrappers > >    > >   Those wroappers are nice, but do not really add much value. > > > > Modified: > >   head/stand/i386/libi386/biosdisk.c > > > > Modified: head/stand/i386/libi386/biosdisk.c > > ===================================================================== > > ========= > > --- head/stand/i386/libi386/biosdisk.c        Wed Aug 15 21:47:03 > > 2018  (r337877) > > +++ head/stand/i386/libi386/biosdisk.c        Wed Aug 15 22:25:05 > > 2018  (r337878) > > @@ -94,10 +94,7 @@ static int nbdinfo = 0; > >   > >  static void bd_io_workaround(struct disk_devdesc *dev); > >   > > -static int bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, > > -    caddr_t dest); > > -static int bd_write(struct disk_devdesc *dev, daddr_t dblk, int > > blks, > > -    caddr_t dest); > > +static int bd_io(struct disk_devdesc *, daddr_t, int, caddr_t, int); > >  static int bd_int13probe(struct bdinfo *bd); > >   > >  static int bd_init(void); > > @@ -506,7 +503,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t > > dblk, s > >       case F_READ: > >               DEBUG("read %d from %lld to %p", blks, dblk, buf); > >   > > -             if (blks && (rc = bd_read(dev, dblk, blks, buf))) { > > +             if (blks && (rc = bd_io(dev, dblk, blks, buf, 0))) { > >                       /* Filter out floppy controller errors */ > >                       if (BD(dev).bd_flags != BD_FLOPPY || rc != > > 0x20) { > >                               printf("read %d from %lld to %p, > > error: 0x%x\n", > > @@ -518,7 +515,7 @@ bd_realstrategy(void *devdata, int rw, daddr_t > > dblk, s > >       case F_WRITE : > >               DEBUG("write %d from %lld to %p", blks, dblk, buf); > >   > > -             if (blks && bd_write(dev, dblk, blks, buf)) { > > +             if (blks && bd_io(dev, dblk, blks, buf, 1)) { > >                       DEBUG("write error"); > >                       return (EIO); > >               } > > @@ -713,20 +710,6 @@ bd_io(struct disk_devdesc *dev, daddr_t dblk, > > int blks > >       } > >   > >       return (0); > > -} > > - > > -static int > > -bd_read(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t > > dest) > > -{ > > - > > -     return (bd_io(dev, dblk, blks, dest, 0)); > > -} > > - > > -static int > > -bd_write(struct disk_devdesc *dev, daddr_t dblk, int blks, caddr_t > > dest) > > -{ > > - > > -     return (bd_io(dev, dblk, blks, dest, 1)); > >  } > >   > >  /* > > > > This would be a more satisfying change if there were something like > >  #define BD_RD 0 >  #define BD_WR 1 > > so that it was clear at a glance what a bd_io() call is doing. > > > I think that's a good idea... Arguably the bd_read/write wrappers were even clearer (and there purpose was readability in that case). -- John Baldwin