On Fri, 2009-06-19 at 20:44 +0200, Vladimir 'phcoder' Serbinenko wrote: > I see the standard is grub_error(). Let's do it for SCSI as > well. > > I don't understand what do you mean. grub_error () which don't come > from previous function
You fixed some code in one place, but it's present in more than one place in the same function. Please either do it consistently or explain why it's needed only in one place. Also, I see that .open functions in files under /disk use grub_error() to communicate errors to the caller. Please explain why you want to do it differently in scsi.c. It's possible that the reasons are simple for somebody who reads the code carefully, but if you are submitting the patch, it's important that you demonstrate that you know what you are doing. > Something is wrong with the logic in that function. retrycnt > is only > changed in one place, and if it hits zero, we don't go to the > retry > label. I think the condition can be removed. I 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. The whole point in enabling more code is to catch such bugs and fix them. Fixing just the immediate obstacles makes the whole task pointless, as it hides half or the problems. Admittedly, I choked a warning in the escape sequence processing in serial.c without fixing the escape sequence support, but the fix would require a major rewrite, and I actually posted a message about the problem. > 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 my previous fixes picked it right Good. -- Regards, Pavel Roskin _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel