On Tue, Mar 31, 2015 at 07:13:40PM +0100, Tomas Elf wrote:
> On 19/03/2015 12:31, John.C.Harrison@xxxxxxxxx wrote:
> >From: John Harrison <John.C.Harrison@xxxxxxxxx>
> >@@ -652,8 +653,11 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf,
> > break;
> > }
> >
> >- if (&request->list == &ring->request_list)
> >+ /* It should always be possible to find a suitable request! */
> >+ if (&request->list == &ring->request_list) {
> >+ WARN_ON(true);
>
> I agree with Thomas Daniel's earlier review comment that WARN_ON(true) is
> not very helpful. You could do something like:
>
> WARN_ON(ret = <expression>);
> if (ret)
> return -ENOSPC;
>
> That way you don't have the redundancy of doing
>
> WARN_ON(expression);
> if (expression)
> return -ENOSPC;
>
>
> and you don't have to do if (WARN_ON(expression)) if you don't like it.
if (WARN_ON(expression)) is prefectly acceptable style in i915 (there's
lots more of these) and what I'd go with here.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
