On Tue, Jul 02, 2013 at 09:52:02AM +0900, Seung-Woo Kim wrote:
If raw_edid of drm_edid_block_vaild() is null, it will crash, so checking in bad label is removed and instead assertion is added at the top of the function. The type of return for the function is bool, so it fixes to return true and false instead of 1 and 0.
Signed-off-by: Seung-Woo Kim sw0312.kim@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
chages from v1
- NULL checking is replaced with WARN_ON() as Daniel commented
- all return value is replaced as true/false as Chris and Daniel commented
drivers/gpu/drm/drm_edid.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 2dc1a60..1117f02 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -968,6 +968,8 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid) u8 csum = 0; struct edid *edid = (struct edid *)raw_edid;
- WARN_ON(!raw_edid);
I don't see this buying us anything. All you get is two backtraces instead of one.
if (WARN_ON(!raw_edid)) return false;
would make more sense if we want tolerate programmer errors a bit better. I'm not a huge fan of such defensive programming though since it tends to make it less clear what the function actually expects from you. But here the WARN_ON() would at least make it clear that it's not meant to happen.
if (edid_fixup > 8 || edid_fixup < 0) edid_fixup = 6;
@@ -1010,15 +1012,15 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool print_bad_edid) break; }
- return 1;
- return true;
bad:
- if (raw_edid && print_bad_edid) {
- if (print_bad_edid) { printk(KERN_ERR "Raw EDID:\n"); print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1, raw_edid, EDID_LENGTH, false); }
- return 0;
- return false;
} EXPORT_SYMBOL(drm_edid_block_valid);
-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel