[PATCH -next] drm/tegra: Use PTR_ERR_OR_ZERO in tegra_gem_create()

[Thierry Reding]

On Tue, Sep 25, 2018 at 04:57:30PM +0900, Mikko Perttunen wrote:
> On 25/09/2018 16.37, Julia Lawall wrote:
> > 
> > 
> > On Tue, 25 Sep 2018, Mikko Perttunen wrote:
> > 
> > > I'm not the maintainer, but in line with previous similar patches..
> > > 
> > > NAK: this makes the code harder to read.
> > 
> > If people don't like it, I wonder if it is a good thing for the function
> > to even exist?  Or at least the semantic patch that suggests this could be
> > removed.
> 
> Good question. I think it may still have its place in some situations - e.g.
> if there's only one call, something like
> 
>   return PTR_ERR_OR_ZERO(do_something());
> 
> where this is the only potentially erroring thing in the function.
> 
> In this case (and the previous ones I referred to), it's been a function
> with a longer series of code like
> 
>   variable = function(...);
>   if (IS_ERR(variable))
>     return PTR_ERR(variable);
> 
> and if we just change the last one it looks out of place. Although honestly,
> I would just write the first example in long-form as well.
> 
> In the end it's a question of taste. With Tegra code we have gone for not
> using PTR_ERR_OR_ZERO.

This has come up recently for the Tegra PCI driver as well, and we were
wondering the same thing. I know that Bjorn (PCI maintainer) and I have
in the past agreed that PTR_ERR_OR_ZERO() is not something we like. For
mostly the same reasons that Mikko already cited. In addition I think
that PTR_ERR_OR_ZERO() makes it needlessly complicated to append code
to a function. Instead of just adding a:

	ptr = function(...);
	if (IS_ERR(ptr))
		return PTR_ERR(ptr);

You need to split up the PTR_ERR_OR_ZERO() first and then be careful
that you return the correct result, etc.

I'm sure there are people that prefer PTR_ERR_OR_ZERO() over the open-
coded alternative, so this is probably just something we're going to
have to live with. Perhaps a good compromise would be to keep the macro
around, but get rid of the semantic patch that suggests the change. At
least that way we would have to go over this over and over again.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180925/1d566588/attachment.sig>