From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1MRT18-0006sF-Qd for mharc-grub-devel@gnu.org; Thu, 16 Jul 2009 11:37:06 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MRT17-0006pR-5D for grub-devel@gnu.org; Thu, 16 Jul 2009 11:37:05 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MRT15-0006mE-JU for grub-devel@gnu.org; Thu, 16 Jul 2009 11:37:04 -0400 Received: from [199.232.76.173] (port=39312 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MRT15-0006m6-EV for grub-devel@gnu.org; Thu, 16 Jul 2009 11:37:03 -0400 Received: from ey-out-1920.google.com ([74.125.78.150]:63269) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MRT14-0006GK-Sp for grub-devel@gnu.org; Thu, 16 Jul 2009 11:37:03 -0400 Received: by ey-out-1920.google.com with SMTP id 5so48125eyb.24 for ; Thu, 16 Jul 2009 08:36:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type :content-transfer-encoding; bh=rYDFLfqN3UMl+OrEWplFe9d3fUukTrAHdBYM62QLFfk=; b=ucYZwmqCk1QHENMKRDA7mfJWUQKHFXhP4KeWv092U8TxYcCRDjpE6loz6GOOrSMI1n 3dBPIZRqYFA9zyURYwVvmW9fLRn7B7bNFBEpRnbabZJgNpfxOYqVgwdZXlCjf6rjZZm6 61n/zci8Vda7uEv7Lq3rwVH3iR7KUXzvfgABI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; b=rkjB12deNMpT2nQlXJM1z2XkSHN/YWuMEhuVwKHfokatIwDCrjEkWIuhsZnZafxx85 uT1jzdQG4gaoZrLG1ha9d4OFyyjy1w8o2IRzTjvn8x2ecrfv7qZXpzn9eTSPF/upQMeN oRnKSlRX2QgjZZOi2bCFbzYasgBFCV3uA/V24= MIME-Version: 1.0 Received: by 10.210.57.3 with SMTP id f3mr11075264eba.91.1247758617900; Thu, 16 Jul 2009 08:36:57 -0700 (PDT) In-Reply-To: References: <1245424852.3431.30.camel@mj> <1245430369.28417.46.camel@mj> Date: Thu, 16 Jul 2009 17:36:56 +0200 Message-ID: From: "Vladimir 'phcoder' Serbinenko" To: The development of GRUB 2 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: [PATCH] fix SigSegV and hang with grub-emu-usb X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Jul 2009 15:37:05 -0000 committed On Fri, Jun 19, 2009 at 8:44 PM, Vladimir 'phcoder' Serbinenko wrote: > > > On Fri, Jun 19, 2009 at 6:52 PM, Pavel Roskin wrote: >> >> OK, I understand you tried USB mass storage devices. >> >> I believe the paramount here is consistency. =A0There are several places >> in the code where grub_errno is returned. =A0In one place, grub_error() = is >> returned. =A0It's important to fix all places at once. >> >> Also, please check other .open functions in other disk drivers. =A0In >> disk/fs_uuid.c, grub_error() is used. =A0The same is in disk/host.c. >> >> I see the standard is grub_error(). =A0Let's do it for SCSI as well. > > I don't understand what do you mean. grub_error () which don't come from > previous function >> >> Something is wrong with the logic in that function. =A0retrycnt is only >> changed in one place, and if it hits zero, we don't go to the retry >> label. =A0I think the condition can be removed. =A0I don't see how your >> change could have fixed anything in the behavior. > > Wel it didn't fixed the logic completely just one case when it was wrong. > Sorry that patch was low-quality. My goal was to enable everything by > default and the bugs in long-unmaintained libusb code weren't something I > wanted to spend time on. >> >> > @@ -246,6 +246,7 @@ grub_usbms_transfer (struct grub_scsi *scsi, >> > grub_size_t cmdsize, char *cmd, >> > =A0 =A0 =A0 =A0if (err =3D=3D GRUB_USB_ERR_STALL) >> > =A0 =A0 =A0{ >> > =A0 =A0 =A0 =A0grub_usb_clear_halt (dev->dev, dev->out->endp_addr); >> > + =A0 =A0 =A0retrycnt--; >> > =A0 =A0 =A0 =A0goto retry; >> > =A0 =A0 =A0} >> > =A0 =A0 =A0 =A0return grub_error (GRUB_ERR_IO, "USB Mass Storage reque= st >> > failed");; >> > retrycnt wasn't decreased which caused grub2 to retry infinitely hence >> > a hang. >> >> There are many instances of "goto retry" where you don't decrement >> retrycnt. =A0Then let's decrement retrycnt in the beginning. >> >> Generally, when making a change, please have a look if it needs to be >> done elsewhere. >> >> > --- a/util/usb.c >> > +++ b/util/usb.c >> > @@ -51,6 +51,7 @@ grub_libusb_devices (void) >> > =A0 =A0 =A0 =A0for (usbdev =3D bus->devices; usbdev; usbdev =3D usbdev= ->next) >> > =A0 =A0 =A0{ >> > =A0 =A0 =A0 =A0struct usb_device_descriptor *desc =3D &usbdev->descrip= tor; >> > + =A0 =A0 =A0grub_err_t err; >> > >> > =A0 =A0 =A0 =A0if (! desc->bcdUSB) >> > =A0 =A0 =A0 =A0 =A0continue; >> > @@ -62,7 +63,9 @@ grub_libusb_devices (void) >> > =A0 =A0 =A0 =A0dev->data =3D usbdev; >> > >> > =A0 =A0 =A0 =A0/* Fill in all descriptors. =A0*/ >> > - =A0 =A0 =A0grub_usb_device_initialize (dev); >> > + =A0 =A0 =A0err =3D grub_usb_device_initialize (dev); >> > + =A0 =A0 =A0if (err) >> > + =A0 =A0 =A0 =A0continue; > > It was clearing grub_errno >> >> >> > =A0 =A0 =A0 =A0 Regarding the compile warning fix, I would try to make >> > =A0 =A0 =A0 =A0 grub_libusb_init() and grub_libusb_fini() appear in >> > =A0 =A0 =A0 =A0 grub_emu_init.h >> > =A0 =A0 =A0 =A0 rather than declare them elsewhere. >> > I was inspired by previous example of disk subsystems: >> > #ifdef GRUB_UTIL >> > void grub_raid_init (void); >> > void grub_raid_fini (void); >> > void grub_lvm_init (void); >> > void grub_lvm_fini (void); >> > #endif >> > file: include/grub/disk.h >> >> I would check why it was needed. > > It seems it's unnecessary. I removed them and it didn't generate any > warnings. Now I followed your recommendation and they build system with m= y > previous fixes picked it right >> >> -- >> Regards, >> Pavel Roskin >> >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel > > > > -- > Regards > Vladimir 'phcoder' Serbinenko > > Personal git repository: http://repo.or.cz/w/grub2/phcoder.git > --=20 Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git