From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57686) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoFpD-000863-Lt for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:24:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SoFp8-0005SD-NR for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:24:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45059) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoFp8-0005Ry-FH for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:24:30 -0400 Message-ID: <4FFAF7A9.8070506@redhat.com> Date: Mon, 09 Jul 2012 17:24:25 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1341843388-5663-1-git-send-email-kwolf@redhat.com> <1341843388-5663-24-git-send-email-kwolf@redhat.com> <4FFAF261.5010804@codemonkey.ws> In-Reply-To: <4FFAF261.5010804@codemonkey.ws> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 23/25] fdc: Move floppy geometry guessing back from block.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org, Markus Armbruster Am 09.07.2012 17:01, schrieb Anthony Liguori: > On 07/09/2012 09:16 AM, Kevin Wolf wrote: >> From: Markus Armbruster >> >> Commit 5bbdbb46 moved it to block.c because "other geometry guessing >> functions already reside in block.c". Device-specific functionality >> should be kept in device code, not the block layer. Move it back. >> >> Disk geometry guessing is still in block.c. To be moved out in a >> later patch series. >> >> Bonus: the floppy type used in pc_cmos_init() now obviously matches >> the one in the FDrive. Before, we relied on >> bdrv_get_floppy_geometry_hint() picking the same type both in >> fd_revalidate() and in pc_cmos_init(). >> >> Signed-off-by: Markus Armbruster >> Signed-off-by: Kevin Wolf >> diff --git a/hw/pc.c b/hw/pc.c >> index c7e9ab3..e5e7647 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -335,10 +335,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t= above_4g_mem_size, >> ISADevice *floppy, BusState *idebus0, BusState *id= ebus1, >> ISADevice *s) >> { >> - int val, nb, nb_heads, max_track, last_sect, i; >> - FDriveType fd_type[2] =3D { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; >> - FDriveRate rate; >> - BlockDriverState *fd[MAX_FD]; >> + int val, nb, i; >> + FDriveType fd_type[2]; >=20 > This results in: >=20 > CC i386-softmmu/hw/i386/../pc.o > /home/anthony/git/qemu/hw/i386/../pc.c: In function =91pc_cmos_init=92: > /home/anthony/git/qemu/hw/i386/../pc.c:339:16: error: =91fd_type[1]=92 = may be used=20 > uninitialized in this function [-Werror=3Duninitialized] > /home/anthony/git/qemu/hw/i386/../pc.c:339:16: error: =91fd_type[0]=92 = may be used=20 > uninitialized in this function [-Werror=3Duninitialized] > cc1: all warnings being treated as errors >=20 > And GCC is right as: >=20 >> static pc_cmos_init_late_arg arg; >> >> /* various important CMOS locations needed by PC/Bochs bios */ >> @@ -381,13 +379,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t= above_4g_mem_size, >> >> /* floppy type */ >> if (floppy) { >> - fdc_get_bs(fd, floppy); >> for (i =3D 0; i< 2; i++) { >> - if (fd[i]) { >> - bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_tr= ack, >> -&last_sect, FDRIVE_DRV_NONE, >> -&fd_type[i],&rate); >> - } >> + fd_type[i] =3D isa_fdc_get_drive_type(floppy, i); >> } >> } >> val =3D (cmos_get_fd_drive_type(fd_type[0])<< 4) | >=20 > This is an unconditional use of fd_type[0]. If floppy =3D=3D NULL, thi= s is=20 > dereferencing an uninitialized value. >=20 > I'm not sure why the explicit initialization was removed... Looks broken indeed. I just wonder why my gcc (or the buildbots) didn't complain. I dropped this patch from for-anthony, so you can give the pull request another try. Kevin