From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46850) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoG9l-0002EY-8x for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:45:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SoG9c-0002vH-K3 for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:45:48 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:62969) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SoG9c-0002uE-Bm for qemu-devel@nongnu.org; Mon, 09 Jul 2012 11:45:40 -0400 Received: by yenl1 with SMTP id l1so11057654yen.4 for ; Mon, 09 Jul 2012 08:45:38 -0700 (PDT) Message-ID: <4FFAFC9E.8070600@codemonkey.ws> Date: Mon, 09 Jul 2012 10:45:34 -0500 From: Anthony Liguori 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> <4FFAF7A9.8070506@redhat.com> In-Reply-To: <4FFAF7A9.8070506@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit 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: Kevin Wolf Cc: qemu-devel@nongnu.org, Markus Armbruster On 07/09/2012 10:24 AM, Kevin Wolf wrote: > 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 *idebus1, >>> ISADevice *s) >>> { >>> - int val, nb, nb_heads, max_track, last_sect, i; >>> - FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; >>> - FDriveRate rate; >>> - BlockDriverState *fd[MAX_FD]; >>> + int val, nb, i; >>> + FDriveType fd_type[2]; >> >> This results in: >> >> CC i386-softmmu/hw/i386/../pc.o >> /home/anthony/git/qemu/hw/i386/../pc.c: In function ‘pc_cmos_init’: >> /home/anthony/git/qemu/hw/i386/../pc.c:339:16: error: ‘fd_type[1]’ may be used >> uninitialized in this function [-Werror=uninitialized] >> /home/anthony/git/qemu/hw/i386/../pc.c:339:16: error: ‘fd_type[0]’ may be used >> uninitialized in this function [-Werror=uninitialized] >> cc1: all warnings being treated as errors >> >> And GCC is right as: >> >>> 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 = 0; i< 2; i++) { >>> - if (fd[i]) { >>> - bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_track, >>> -&last_sect, FDRIVE_DRV_NONE, >>> -&fd_type[i],&rate); >>> - } >>> + fd_type[i] = isa_fdc_get_drive_type(floppy, i); >>> } >>> } >>> val = (cmos_get_fd_drive_type(fd_type[0])<< 4) | >> >> This is an unconditional use of fd_type[0]. If floppy == NULL, this is >> dereferencing an uninitialized value. >> >> 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. Okay, am building now and testing. Looks good so far. Regards, Anthony Liguori > > Kevin