Re: [PATCH i-g-t 2/3] lib/kms: Simplify force_connectors[] error handling

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 11, 2024 at 10:25:11AM +0200, Kamil Konieczny wrote:
> Hi Ville,
> On 2024-04-10 at 18:02:28 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The failure to allocate a forced_connector entry is entirely
> > our own fault. If we hit this then we've screwd up and made
> > the array too small. Skip the error handling and just assert
> > that we must have enough room in the array.
> > 
> > Cc: Kunal Joshi <kunal1.joshi@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  lib/igt_kms.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index f7dd1db91827..19bb4ac66ece 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1518,8 +1518,7 @@ static struct igt_forced_connector *forced_connector_alloc(void)
> >  	for (i = 0; forced_connectors[i].connector_type; i++)
> >  		;
> >  
> > -	if (i >= ARRAY_SIZE(forced_connectors))
> > -		return NULL;
> > +	igt_assert_lt(i, ARRAY_SIZE(forced_connectors));
> 
> Asserts in libs are ugly, could we just realloc it once?

We never realloc anything. It's a static array.

> Btw (i == ARRAY_SIZE(forced_connectors) should be ok? Or do
> we treat this as a end-of-array guard?

That's past the end of the array.

> 
> If you insist on keeping assert please at least give a message
> for ease of debug.

Looks like I actually dropped the assert entirely in the next patch
and let it explode naturally. Which it actually did in CI because
I made a booboo in _alloc()...

> 
> Regards,
> Kamil
> 
> >  
> >  	return &forced_connectors[i];
> >  }
> > @@ -1558,11 +1557,6 @@ static bool force_connector(int drm_fd,
> >  	}
> >  
> >  	c = forced_connector_alloc();
> > -	if (!c) {
> > -		igt_warn("Connector limit reached, %s will not be reset\n", name);
> > -		close(dir);
> > -		return true;
> > -	}
> >  
> >  	c->idx = idx;
> >  	c->connector_type = connector->connector_type;
> > -- 
> > 2.43.2
> > 

-- 
Ville Syrjälä
Intel