From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48930 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PJqPL-0002DB-Kd for qemu-devel@nongnu.org; Sat, 20 Nov 2010 11:35:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PJqPK-0003Ca-9Q for qemu-devel@nongnu.org; Sat, 20 Nov 2010 11:35:23 -0500 Received: from mail-vw0-f45.google.com ([209.85.212.45]:46028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PJqPK-0003CC-5Q for qemu-devel@nongnu.org; Sat, 20 Nov 2010 11:35:22 -0500 Received: by vws5 with SMTP id 5so2991784vws.4 for ; Sat, 20 Nov 2010 08:35:20 -0800 (PST) Message-ID: <4CE72952.1010703@codemonkey.ws> Date: Fri, 19 Nov 2010 19:50:10 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v2] ioport: Fix duplicated code References: <20101111120326.22020026@doriath> In-Reply-To: <20101111120326.22020026@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org, gleb@redhat.com, Markus Armbruster On 11/11/2010 08:03 AM, Luiz Capitulino wrote: > Functions register_ioport_read() and register_ioport_write() are almost > identical, the only difference is that they write to different arrays. > > Introduce register_ioport_rw() to handle this difference and change both > functions to use it instead of duplicating code. > > Signed-off-by: Luiz Capitulino > While it make take some scripting, let's do a global query/replace. Having two interfaces where one is only scarely used hurts code readability. We need to do the janitorial work when changing interfaces like this. Regards, Anthony Liguori > --- > > v2: Fix error messages and make register_ioport_rw() register both handlers > at the same call > > ioport.c | 37 ++++++++++++++++++------------------- > 1 files changed, 18 insertions(+), 19 deletions(-) > > diff --git a/ioport.c b/ioport.c > index ec3dc65..4560973 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -137,41 +137,40 @@ static int ioport_bsize(int size, int *bsize) > } > > /* size is the word size in byte */ > -int register_ioport_read(pio_addr_t start, int length, int size, > - IOPortReadFunc *func, void *opaque) > +static int register_ioport_rw(pio_addr_t start, int length, int size, > + IOPortReadFunc *read_func, > + IOPortWriteFunc *write_func, void *opaque) > { > int i, bsize; > > if (ioport_bsize(size,&bsize)) { > - hw_error("register_ioport_read: invalid size"); > + hw_error("register_ioport_rw: invalid size"); > return -1; > } > for(i = start; i< start + length; i += size) { > - ioport_read_table[bsize][i] = func; > + if (read_func) { > + ioport_read_table[bsize][i] = read_func; > + } > + if (write_func) { > + ioport_write_table[bsize][i] = write_func; > + } > if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > - hw_error("register_ioport_read: invalid opaque"); > + hw_error("register_ioport_rw: invalid opaque"); > ioport_opaque[i] = opaque; > } > return 0; > } > > -/* size is the word size in byte */ > +int register_ioport_read(pio_addr_t start, int length, int size, > + IOPortReadFunc *func, void *opaque) > +{ > + return register_ioport_rw(start, length, size, func, NULL, opaque); > +} > + > int register_ioport_write(pio_addr_t start, int length, int size, > IOPortWriteFunc *func, void *opaque) > { > - int i, bsize; > - > - if (ioport_bsize(size,&bsize)) { > - hw_error("register_ioport_write: invalid size"); > - return -1; > - } > - for(i = start; i< start + length; i += size) { > - ioport_write_table[bsize][i] = func; > - if (ioport_opaque[i] != NULL&& ioport_opaque[i] != opaque) > - hw_error("register_ioport_write: invalid opaque"); > - ioport_opaque[i] = opaque; > - } > - return 0; > + return register_ioport_rw(start, length, size, NULL, func, opaque); > } > > void isa_unassign_ioport(pio_addr_t start, int length) >