All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Mikko Perttunen <cyndis@kapsi.fi>
Cc: Qinglang Miao <miaoqinglang@huawei.com>,
	dri-devel@lists.freedesktop.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()
Date: Mon, 21 Sep 2020 15:27:24 +0200	[thread overview]
Message-ID: <20200921132724.GC3955907@ulmo> (raw)
In-Reply-To: <4378d69a-2338-779f-ab4d-3c64fbf7dfd3@kapsi.fi>

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

On Mon, Sep 21, 2020 at 04:12:20PM +0300, Mikko Perttunen wrote:
> On 9/21/20 4:10 PM, Qinglang Miao wrote:
> > Simplify the return expression.
> > 
> > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com>
> > ---
> >   drivers/gpu/host1x/cdma.c | 8 +-------
> >   1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> > index e8d3fda91..08a0f9e10 100644
> > --- a/drivers/gpu/host1x/cdma.c
> > +++ b/drivers/gpu/host1x/cdma.c
> > @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
> >    */
> >   int host1x_cdma_init(struct host1x_cdma *cdma)
> >   {
> > -	int err;
> > -
> >   	mutex_init(&cdma->lock);
> >   	init_completion(&cdma->complete);
> > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
> >   	cdma->running = false;
> >   	cdma->torndown = false;
> > -	err = host1x_pushbuffer_init(&cdma->push_buffer);
> > -	if (err)
> > -		return err;
> > -
> > -	return 0;
> > +	return host1x_pushbuffer_init(&cdma->push_buffer);
> 
> IMHO, this makes it less readable since now it kind of looks like
> host1x_pushbuffer_init is returning some meaningful value, instead of the
> code just handling error values in a sequence.

Agreed. Collapsing the error handling like this also makes adding code
to the bottom of this function more complicated than necessary.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding@gmail.com>
To: Mikko Perttunen <cyndis@kapsi.fi>
Cc: linux-tegra@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Qinglang Miao <miaoqinglang@huawei.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()
Date: Mon, 21 Sep 2020 15:27:24 +0200	[thread overview]
Message-ID: <20200921132724.GC3955907@ulmo> (raw)
In-Reply-To: <4378d69a-2338-779f-ab4d-3c64fbf7dfd3@kapsi.fi>


[-- Attachment #1.1: Type: text/plain, Size: 1432 bytes --]

On Mon, Sep 21, 2020 at 04:12:20PM +0300, Mikko Perttunen wrote:
> On 9/21/20 4:10 PM, Qinglang Miao wrote:
> > Simplify the return expression.
> > 
> > Signed-off-by: Qinglang Miao <miaoqinglang@huawei.com>
> > ---
> >   drivers/gpu/host1x/cdma.c | 8 +-------
> >   1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> > index e8d3fda91..08a0f9e10 100644
> > --- a/drivers/gpu/host1x/cdma.c
> > +++ b/drivers/gpu/host1x/cdma.c
> > @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
> >    */
> >   int host1x_cdma_init(struct host1x_cdma *cdma)
> >   {
> > -	int err;
> > -
> >   	mutex_init(&cdma->lock);
> >   	init_completion(&cdma->complete);
> > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
> >   	cdma->running = false;
> >   	cdma->torndown = false;
> > -	err = host1x_pushbuffer_init(&cdma->push_buffer);
> > -	if (err)
> > -		return err;
> > -
> > -	return 0;
> > +	return host1x_pushbuffer_init(&cdma->push_buffer);
> 
> IMHO, this makes it less readable since now it kind of looks like
> host1x_pushbuffer_init is returning some meaningful value, instead of the
> code just handling error values in a sequence.

Agreed. Collapsing the error handling like this also makes adding code
to the bottom of this function more complicated than necessary.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-09-21 13:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-21 13:10 [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init() Qinglang Miao
2020-09-21 13:10 ` Qinglang Miao
2020-09-21 13:12 ` Mikko Perttunen
2020-09-21 13:12   ` Mikko Perttunen
2020-09-21 13:27   ` Thierry Reding [this message]
2020-09-21 13:27     ` Thierry Reding
2020-09-22  2:33   ` miaoqinglang
2020-09-22  2:33     ` miaoqinglang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200921132724.GC3955907@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=cyndis@kapsi.fi \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=miaoqinglang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.