From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Dave Airlie <airlied@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 13/15] drm: Eliminate pointless goto
Date: Thu, 15 Mar 2012 19:17:13 +0200 [thread overview]
Message-ID: <20120315171713.GL4917@intel.com> (raw)
In-Reply-To: <CAPM=9tyRxU6PTM7AK3GsvXnSPSWZt+3ADhT450Eb-RRHYtPk0Q@mail.gmail.com>
On Thu, Mar 15, 2012 at 09:57:53AM +0000, Dave Airlie wrote:
> > Use a do {} while() loop instead of a goto in drm_mode_object_get().
>
> I don't like this one just because it diverges our idr usage from the
> canonical idr usage at
>
> http://lwn.net/Articles/103209/
>
> So unless there is a good reason for the change I'd rather not apply it.
That's fine by me. Although "canonical" may be an overstatement. I just
quickly went through the idr_pre_get() usage in the tree, and between
the three conteders (no loop, goto loop, while loop) it was a close
race. No loop won with 29 idr_pre_get()s, goto loop came in second
with 24, and while loop came in third with 21.
The drm subsystem itself seems to have 5 goto loops. So if we were to
change them all, goto loop would lose to the while loop.
> I've applied the other patches except this and the last one which I
> think depends on it.
The error handling in the goto version is equally broken. I'll respin
the patch on top of the goto version, and I'll check the other idr
usage under drm as well...
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2012-03-15 17:09 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-13 10:35 [PATCH 00/15] drm: Bounds checking, error handling, etc ville.syrjala
2012-03-13 10:35 ` [PATCH 01/15] drm: Reject mode set with current fb if no current fb is bound ville.syrjala
2012-03-13 10:35 ` [PATCH 02/15] drm: Change drm_display_mode::type to unsigned ville.syrjala
2012-03-13 10:35 ` [PATCH 03/15] drm: Warn if mode to umode conversion overflows the destination types ville.syrjala
2012-03-13 10:35 ` [PATCH 04/15] drm: Check crtc x and y coordinates ville.syrjala
2012-03-13 10:35 ` [PATCH 05/15] drm: Make drm_mode_attachmode() void ville.syrjala
2012-03-13 10:35 ` [PATCH 06/15] drm: Fix memory leak in drm_mode_setcrtc() ville.syrjala
2012-03-13 10:35 ` [PATCH 07/15] drm: Check user mode against overflows ville.syrjala
2012-03-13 10:35 ` [PATCH 08/15] drm: Check CRTC viewport against framebuffer size ville.syrjala
2012-03-13 10:35 ` [PATCH 09/15] drm: Fix drm_mode_attachmode_crtc() ville.syrjala
2012-03-13 10:35 ` [PATCH 10/15] drm: Make drm_crtc_convert_{umode, to_umode} static and constify their params ville.syrjala
2012-03-13 10:35 ` [PATCH 11/15] drm: Handle drm_object_get() failures ville.syrjala
2012-03-13 10:35 ` [PATCH 12/15] drm: Use a flexible array member for blob property data ville.syrjala
2012-03-13 10:35 ` [PATCH 13/15] drm: Eliminate pointless goto ville.syrjala
2012-03-15 9:57 ` Dave Airlie
2012-03-15 17:17 ` Ville Syrjälä [this message]
2012-03-13 10:35 ` [PATCH 14/15] drm: Add drm_mode_copy() ville.syrjala
2012-03-13 10:35 ` [PATCH 15/15] drm: Fix drm_mode_objecte_get() return values ville.syrjala
2012-03-13 13:13 ` [PATCH 00/15] drm: Bounds checking, error handling, etc Alex Deucher
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=20120315171713.GL4917@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=airlied@gmail.com \
--cc=dri-devel@lists.freedesktop.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.