lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 19 May 2014 15:39:58 -0400 (EDT) From: Alan Stern <stern@...land.harvard.edu> To: Ricardo Ribalda Delgado <ricardo.ribalda@...il.com> cc: Felipe Balbi <balbi@...com>, <gregkh@...uxfoundation.org>, Kernel development list <linux-kernel@...r.kernel.org>, USB list <linux-usb@...r.kernel.org>, <joe@...ches.com>, <auttamchandani@...icube.com> Subject: Re: [PATCH 3/5] usb: gadget: net2280: Pass checkpacth.pl test On Mon, 19 May 2014, Ricardo Ribalda Delgado wrote: > Fix Code Style using checkpatch.pl criteria > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@...il.com> Not really necessary, although this doesn't hurt. It's notable that scattered in amongst all the checkpatch-related changes are a few things that actually alter the meaning of the code. Mixing things up like that isn't a good idea. Also, there seem to be several questionable changes: > @@ -236,7 +234,7 @@ net2280_enable (struct usb_ep *_ep, const struct usb_endpoint_descriptor *desc) > return -ERANGE; > } > } > - ep->is_iso = (tmp == USB_ENDPOINT_XFER_ISOC) ? 1 : 0; > + ep->is_iso = !!(tmp == USB_ENDPOINT_XFER_ISOC); Getting rid of the "? 1 : 0" part is fine, but why introduce the "!!"? Don't you realize that the equality test will always produce a 0 or 1 result? > > /*-------------------------------------------------------------------------*/ > > -static struct usb_request * > -net2280_alloc_request (struct usb_ep *_ep, gfp_t gfp_flags) > +static struct usb_request *net2280_alloc_request(struct usb_ep *_ep, > + gfp_t gfp_flags) What's with the extreme indentation on the continuation line? The style used here is for continuation lines to be indented by two stops relative to the first line. > @@ -613,20 +609,21 @@ write_fifo (struct net2280_ep *ep, struct usb_request *req) > * NOTE: also used in cases where that erratum doesn't apply: > * where the host wrote "too much" data to us. > */ > -static void out_flush (struct net2280_ep *ep) > +static void out_flush(struct net2280_ep *ep) > { > u32 __iomem *statp; > u32 tmp; > > - ASSERT_OUT_NAKING (ep); > + ASSERT_OUT_NAKING(ep); > > statp = &ep->regs->ep_stat; > writel(BIT(DATA_OUT_PING_TOKEN_INTERRUPT) | > BIT(DATA_PACKET_RECEIVED_INTERRUPT) > , statp); > writel(BIT(FIFO_FLUSH), statp); > - mb (); > - tmp = readl (statp); > + /* Make sure stap is written before readded back */ If you're going to add a comment, add least use correct spelling and grammar. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists