Re: may I commit this small umodem patch ?

On Thu, Jul 03, 2008 at 05:07:00PM +0200, Gary Jennejohn wrote:
> On Thu, 3 Jul 2008 16:07:19 +0200
> Luigi Rizzo <rizzo_at_icir.org> wrote:
> 
> > There was a discussion back in september about adding
> > support for basic CDC tty devices in umodem.c.
> > This lets you talk to a number of usb devices built around
> > microcontrollers (e.g. Atmel), and puts us on par with
> > Linux and Windows in terms of supporting these devices.
> > 
> > Because this simply requires the small patch below to the
> > probe/attach routine, so if there are no objections I plan to add
> > this to the system (CURRENT then RELENG_7 and RELENG_6) in the next
> > few days.
> > 
> > > Index: umodem.c
> > > ===================================================================
> > > RCS file: /home/ncvs/src/sys/dev/usb/umodem.c,v
> > > retrieving revision 1.57
> > > diff -u -r1.57 umodem.c
> > > --- umodem.c	31 Jan 2005 13:58:10 -0000	1.57
> > > +++ umodem.c	20 Aug 2006 17:05:34 -0000
> > > _at__at_ -256,6 +260,15 _at__at_
> > >  	    id->bInterfaceProtocol == UIPROTO_CDC_AT)
> > >  		ret = UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO;
> > >  
> > > +#if 1
> > > +	if (ret == UMATCH_NONE &&
> > > +	    id->bInterfaceClass == UICLASS_CDC_DATA &&
> > > +	    id->bInterfaceSubClass == UISUBCLASS_DATA &&
> > > +	    id->bInterfaceProtocol == 0x00)
> > > +		ret = UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO;
> > > +		return ret;
> > > +#endif
> > > +
> > >  	if (ret == UMATCH_NONE)
> > >  		return (ret);
> > 
> 
> Is there any reason to keep the #if 1 ... #endif?  And why not just
> directly return UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO rather than
> assigning it to ret first?

in fact there are also missing braces that need to be
added -- as is, the code after the #endif is completely disabled.

thanks for the comment, but don't worry, the commit will be done
the right way.

cheers
luigi