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@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
