All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
To: Sergei Shtylyov <sshtylyov@mvista.com>
Cc: Julius Werner <jwerner@chromium.org>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Vincent Palatin <vpalatin@chromium.org>
Subject: Re: [PATCH] xhci: fix null-pointer dereference when destroying half-built segment rings
Date: Thu, 1 Nov 2012 10:52:17 -0700	[thread overview]
Message-ID: <20121101175217.GJ29270@xanatos> (raw)
In-Reply-To: <508ECC63.3020302@mvista.com>

On Mon, Oct 29, 2012 at 09:35:15PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/29/2012 08:00 PM, Julius Werner wrote:
> >  		next = xhci_segment_alloc(xhci, cycle_state, flags);
> >  		if (!next) {
> > -			xhci_free_segments_for_ring(xhci, *first);
> > +			prev = *first;
> > +			do {
> > +				next = prev->next;
> > +				xhci_segment_free(xhci, prev);
> > +			} while ((prev = next));
> 
>    It's preferred that the assignments are done outside the *if* and *while*
> statements. In fact, at least for the *if* statements scripts/checkpatch.pl
> gives a warning (it was silent in this case).

Hi Julius,

I agree with Sergei (for once).  The assignment in the while conditional
is confusing, and everyone reading the code will wonder if you meant
(prev == next).  Putting extra parenthesis around it to avoid static
type checker warnings is kind of missing the point.

Your patch does apply fine with git-am, so don't worry about the line
endings.  Can you please move the assignment into the loop and resubmit
this patch?

Sarah Sharp

  parent reply	other threads:[~2012-11-01 17:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-29 17:00 [PATCH] xhci: fix null-pointer dereference when destroying half-built segment rings Julius Werner
2012-10-29 18:35 ` Sergei Shtylyov
2012-10-29 18:54   ` Julius Werner
2012-11-01 17:52   ` Sarah Sharp [this message]
2012-11-01 19:47     ` Julius Werner
2012-11-01 20:13       ` Andy Shevchenko
2012-11-01 20:15         ` Sarah Sharp
2012-11-01 20:28           ` Julius Werner
2012-11-12 18:03             ` Sarah Sharp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121101175217.GJ29270@xanatos \
    --to=sarah.a.sharp@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jwerner@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=sshtylyov@mvista.com \
    --cc=vpalatin@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.