[PATCH i-g-t 1/2] lib/igt_chamelium: check drmModeGetConnector() return value in wait_for_connected_state()

Luca Coelho luca at coelho.fi
Tue May 28 07:20:45 UTC 2024


On Fri, 2024-05-24 at 18:58 +0200, Kamil Konieczny wrote:
> Hi Luca,

Hi Kamil,


> On 2024-05-23 at 11:44:25 +0300, Luca Coelho wrote:
> 
> I have two nits, one about too long subject:
> 
> [PATCH i-g-t 1/2] lib/igt_chamelium: check drmModeGetConnector() return value in wait_for_connected_state()

Yes, it's a bit too long, but I wanted to convey useful information in
the subject...


> imho better:
> 
> [PATCH i-g-t 1/2] lib/igt_chamelium: avoid NULL dereference in wait_for_connected_state()

Okay, this is descriptive enough, but loses the context of the problem
with drmModeGetConnector(), which I'm addressing in this series.


> or even shorter:
> 
> [PATCH i-g-t 1/2] lib/igt_chamelium: fix wait_for_connected_state()

IMHO this is way too generic.

What about this?

[PATCH i-g-t 1/2] lib/igt_chamelium: check drmModeGetConnector() return in wait_for_connected_state()

It's still sligthly over 80 chars, but conveys the full message (I just
remove "value" from it).  And the max 80 chars rule is so outdated that
it's not very strict even in the kernel nowadays...

> Maybe that should be changed to something else? See my other
> response.
> 
> > The drmModeGetConnector() function can return NULL in some cases, so
> > we need to check the return value before accessing it.  This is not
> > being checked in wait_for_connected_state(), so fix that.
> > 
> 
> Also please add to Cc chromium devs, for example:
> 
> Cc: Mark Yacoub <markyacoub at chromium.org>
> Cc: Manasi Navare <navaremanasi at chromium.org>
> > Signed-off-by: Luca Coelho <luciano.coelho at intel.com>

Done.  Thanks for pointing out!

--
Cheers,
Luca.


More information about the igt-dev mailing list