On Wed, Mar 23, 2011 at 04:32:57PM -0700, Dmitry Torokhov wrote: > Hi SaÑÐh, > > On Wed, Mar 23, 2011 at 03:10:09PM -0700, Sarah Sharp wrote: > > Hi Dmitry, > > > > Can you please respin this patch to not include initialization in > > variable declarations? In general, I'd like to avoid anything but > > simple initialization for things like setting return values to 0 or > > pointers to NULL, although I know some of the code in the xHCI driver is > > not consistent about that. > > > > I especially want to avoid initialization-during-declaration lines that > > include function calls, because it makes it really hard to notice what's > > happening in the code. > > I really do not seemuch difference between: > > struct xhci_ring *ep_ring; > struct urb_priv *urb_priv; > int idx; > struct usb_iso_packet_descriptor *frame; > > ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer); > urb_priv = td->urb->hcpriv; > idx = urb_priv->td_cnt; > frame = &td->urb->iso_frame_desc[idx]; > > and: > > struct xhci_ring *ep_ring = xhci_dma_to_transfer_ring(ep, event->buffer); > struct urb_priv *urb_priv = td->urb->hcpriv; > int idx = urb_priv->td_cnt; > struct usb_iso_packet_descriptor *frame = &td->urb->iso_frame_desc[idx]; The first style looks much cleaner to me. I can clearly see what the code is actually doing, without having to mentally wide through the variable types. It just makes catching bugs easier. Please change it. > except that former takes twice as many lines on the screen. Such style > is very common in the kernel code. But I guess you are the boss, if you > insist I can change it back. Yeah, I know it's a pain to respin patches, but I'm going to be maintaining this code for god-knows-how-long and I would like to be able to easily read it. :) Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
