From: Dan Carpenter <dan.carpenter@oracle.com>
To: Colin Ian King <colin.king@canonical.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
James Wang <james.qian.wang@arm.com>,
Brian Starkey <brian.starkey@arm.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done
Date: Mon, 07 Oct 2019 13:25:05 +0000 [thread overview]
Message-ID: <20191007132505.GV22609@kadam> (raw)
In-Reply-To: <35232014-f65a-f7a1-99db-8ed91f610a77@canonical.com>
On Fri, Oct 04, 2019 at 10:53:44PM +0100, Colin Ian King wrote:
> On 04/10/2019 20:27, Liviu Dudau wrote:
> > On Fri, Oct 04, 2019 at 05:21:56PM +0100, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The pointer disable_done is being initialized with a value that
> >> is never read and is being re-assigned a little later on. The
> >> assignment is redundant and hence can be removed.
> >
> > Not really true, isn't it? The re-assignment is done under the condition that
> > crtc->state->active is true. disable_done will be used regardless after the if
> > block, so we can't skip this initialisation.
> >
> > Not sure why Coverity flags this, but I would NAK this patch.
>
> I'm patching against the driver from linux-next so I believe this is OK
> for that. I believe your statement is true against linux which does not
> have commit:
>
> d6cb013579e743bc7bc5590ca35a1943f2b8f3c8
> Author: Lowry Li (Arm Technology China) <Lowry.Li@arm.com>
> Date: Fri Sep 6 07:18:06 2019 +0000
>
It really does help reviewing patches when this is mentioned in the
commit message.
There is some debate about whether this should be mentioned as a Fixes
since it doesn't fix a bug. I initialy felt it shouldn't be, but now
I think enough people think it should be listed as Fixes that I must be
wrong. Either way, it's very useful information.
The other thing is that soon get_maintainer.pl will start CC'ing people
from the Fixes tag and right now Lowry Li is not CC'd so that's
unfortunate.
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Colin Ian King <colin.king@canonical.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
James Wang <james.qian.wang@arm.com>,
Brian Starkey <brian.starkey@arm.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done
Date: Mon, 7 Oct 2019 16:25:05 +0300 [thread overview]
Message-ID: <20191007132505.GV22609@kadam> (raw)
In-Reply-To: <35232014-f65a-f7a1-99db-8ed91f610a77@canonical.com>
On Fri, Oct 04, 2019 at 10:53:44PM +0100, Colin Ian King wrote:
> On 04/10/2019 20:27, Liviu Dudau wrote:
> > On Fri, Oct 04, 2019 at 05:21:56PM +0100, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The pointer disable_done is being initialized with a value that
> >> is never read and is being re-assigned a little later on. The
> >> assignment is redundant and hence can be removed.
> >
> > Not really true, isn't it? The re-assignment is done under the condition that
> > crtc->state->active is true. disable_done will be used regardless after the if
> > block, so we can't skip this initialisation.
> >
> > Not sure why Coverity flags this, but I would NAK this patch.
>
> I'm patching against the driver from linux-next so I believe this is OK
> for that. I believe your statement is true against linux which does not
> have commit:
>
> d6cb013579e743bc7bc5590ca35a1943f2b8f3c8
> Author: Lowry Li (Arm Technology China) <Lowry.Li@arm.com>
> Date: Fri Sep 6 07:18:06 2019 +0000
>
It really does help reviewing patches when this is mentioned in the
commit message.
There is some debate about whether this should be mentioned as a Fixes
since it doesn't fix a bug. I initialy felt it shouldn't be, but now
I think enough people think it should be listed as Fixes that I must be
wrong. Either way, it's very useful information.
The other thing is that soon get_maintainer.pl will start CC'ing people
from the Fixes tag and right now Lowry Li is not CC'd so that's
unfortunate.
regards,
dan carpenter
next prev parent reply other threads:[~2019-10-07 13:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 16:21 [PATCH][next] drm/komeda: remove redundant assignment to pointer disable_done Colin King
2019-10-04 16:21 ` Colin King
2019-10-04 19:27 ` Liviu Dudau
2019-10-04 19:27 ` Liviu Dudau
2019-10-04 21:53 ` Colin Ian King
2019-10-04 21:53 ` Colin Ian King
2019-10-07 13:25 ` Dan Carpenter [this message]
2019-10-07 13:25 ` Dan Carpenter
2019-10-08 8:06 ` james qian wang (Arm Technology China)
2019-10-08 8:06 ` james qian wang (Arm Technology China)
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=20191007132505.GV22609@kadam \
--to=dan.carpenter@oracle.com \
--cc=airlied@linux.ie \
--cc=brian.starkey@arm.com \
--cc=colin.king@canonical.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=james.qian.wang@arm.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=liviu.dudau@arm.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.