[PATCH] drm/i915/display: use IS_ERR_OR_NULL macro on DP tunnel mgr creation failure

[Krzysztof Karas]

Thanks for review!

> > --- a/drivers/gpu/drm/i915/display/intel_dp_tunnel.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_tunnel.c
> > @@ -793,7 +793,7 @@ int intel_dp_tunnel_mgr_init(struct intel_display *display)
> >  	drm_connector_list_iter_end(&connector_list_iter);
> >  
> >  	tunnel_mgr = drm_dp_tunnel_mgr_create(display->drm, dp_connectors);
> > -	if (IS_ERR(tunnel_mgr))
> > +	if (IS_ERR_OR_NULL(tunnel_mgr))
> >  		return PTR_ERR(tunnel_mgr);
> 
> this still will not work as expected, since in case of NULL it will
> return 0 (success) instead of "a negative error code" as described in
> the documentation of the intel_dp_tunnel_mgr_init()
Good catch, we should not return 0 here then.

> 
> OTOH the documentation of drm_dp_tunnel_mgr_create() says: "Returns a
> pointer to the tunnel manager if created successfully or NULL in case of
> an error" so more appropriate fix seems to be:
> 
> -	if (IS_ERR(tunnel_mgr))
> - 		return PTR_ERR(tunnel_mgr);
> +	if (!tunnel_mgr)
> + 		return -ENOMEM;
> 
> but then it will not work with the drm_dp_tunnel_mgr_create() stub which
> actually returns undocumented ERR_PTR(-EOPNOTSUPP)
> 
> so unless you are ready to update implementation and documentation of
> the drm_dp_tunnel_mgr_create() to return ERR_PTR instead of NULL in case
> of error
I considered that and I think this would be overall a better solution,
but as I understand functions in drm_dp_tunnel.c file generally try to
return NULLs, whenever kzalloc/kcalloc fails to allocate, so we'd have
that one odd out here. Though, other ones are 'static', so maybe there
is no need for concern, as they are not going to be exposed.

I'll update drm_dp_tunnel_mgr_create() to return error pointers.

>, the fix IMO should look more like:
> 
> +	if (!tunnel_mgr)
> + 		return -ENOMEM;
> 
> and keep existing IS_ERR check
I do not think it is good to have the caller assume error code from just
a generic NULL. If anything changes in drm_dp_tunnel_mgr_create() and
something else than allocation would be allowed to fail, then ENOMEM
would no longer be appropriate here.


Krzysztof Karas

> 
> >  
> >  	display->dp_tunnel_mgr = tunnel_mgr;
>