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 -- Luiz Fernando N. Capitulino _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
