On Tue, Jun 20, 2006 at 06:30:08PM -0300, Luiz Fernando N. Capitulino wrote: > On Tue, 20 Jun 2006 12:54:48 -0700 > Greg KH <[EMAIL PROTECTED]> wrote: > > | On Tue, Jun 20, 2006 at 12:25:58PM -0700, Pete Zaitcev wrote: > | > On Mon, 19 Jun 2006 17:23:06 -0300, [EMAIL PROTECTED] wrote: > | > > | > > The goal is to make usb_serial_probe() smaller, clean and easy to > follow. > | > > | > I'd like to challenge this assertion. The diffstat is not favourable: > | > > | > > drivers/usb/serial/usb-serial.c | 176 > +++++++++++++++++++++++++++------------ > | > > 1 files changed, 120 insertions(+), 56 deletions(-) > | > | I agree, I don't see the benifit here. > | > | Sure, that probe function does a _lot_ and has a zillion local > | variables. But you didn't reduce the number of local variables, and > | caused a few double error reports like Pete pointed out... > | > | What is the main reason for doing this? > > To make usb_serial_probe() smaller, clean and easy to follow. > > Look, why does get_free_serial() is needed? why not put it's code > block inlined in usb_serial_probe()? Ditto for search_serial_device() > and create_serial(). > > I think moving self-contained code blocks like these ones to > functions helps to clarify what we're doing and why. Which _may_ > also help to find and fix bugs. > > I do agree that the diffstat is not favourable, however, the goal > here is not to save lines of code. That kind of factorization will > be necessary even if we only move to functions the needed code to > kill the automatic variables, because even with it usb_serial_probe() > will be big. > > But I agree about the double error reports. Looking at it now > I can't tell you why they are there. :P
Ok, care to redo the series with this stuff fixed up? thanks, greg k-h _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
