DriverDev-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Cengiz Can <cengiz@kernel.wtf>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH] staging: atomisp: move null check to earlier point
Date: Thu, 6 Aug 2020 17:15:37 -0500
Message-ID: <20200806221537.GA703560@bjorn-Precision-5520> (raw)
In-Reply-To: <20200730084545.GB1793@kadam>

On Thu, Jul 30, 2020 at 11:45:45AM +0300, Dan Carpenter wrote:
> On Wed, Jul 29, 2020 at 06:13:44PM +0300, Andy Shevchenko wrote:
> > On Wed, Jul 29, 2020 at 5:00 PM Cengiz Can <cengiz@kernel.wtf> wrote:
> > >
> > > `find_gmin_subdev` function that returns a pointer to `struct
> > > gmin_subdev` can return NULL.
> > >
> > > In `gmin_v2p8_ctrl` there's a call to this function but the possibility
> > > of a NULL was not checked before its being dereferenced. ie:
> > >
> > > ```
> > > /* Acquired here --------v */
> > > struct gmin_subdev *gs = find_gmin_subdev(subdev);
> > > int ret;
> > > int value;
> > >
> > > /*  v------Dereferenced here */
> > > if (gs->v2p8_gpio >= 0) {
> > >         pr_info("atomisp_gmin_platform: 2.8v power on GPIO %d\n",
> > >                 gs->v2p8_gpio);
> > >         ret = gpio_request(gs->v2p8_gpio, "camera_v2p8");
> > >         if (!ret)
> > >                 ret = gpio_direction_output(gs->v2p8_gpio, 0);
> > >         if (ret)
> > >                 pr_err("V2P8 GPIO initialization failed\n");
> > > }
> > > ```
> > >
> > > I have moved the NULL check before deref point.
> > 
> > "Move the NULL check..."
> > See Submitting Patches documentation how to avoid "This patch", "I", "we", etc.
> 
> I always feel like this is a pointless requirement.  We're turning
> into bureaucrats.

There is a danger of that, and I'm more guilty than most.  But I do
think there's value in consistent style because it allows readers to
focus on the content instead of being distracted by different margins,
grammar ("move vs. moved"), paragraph styles, quoting conventions,
etc.

Ideally we would scan previous commit logs (and the existing code!)
and make new changes fit seamlessly so it looks like everything was
done at the same time by the same person.

But often that doesn't happen.  Sometimes I take the liberty to tweak
things as I apply them to try to avoid trivial rework.

Bjorn
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

  parent reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 13:56 Cengiz Can
2020-07-29 15:13 ` Andy Shevchenko
2020-07-30  8:45   ` Dan Carpenter
2020-07-30  8:59     ` Cengiz Can
2020-07-30 22:17     ` [PATCH v2] " Cengiz Can
2020-07-31  8:38       ` Andy Shevchenko
2020-08-01 21:51         ` [PATCH v3] " Cengiz Can
2020-08-01 21:55         ` [PATCHi v4] " Cengiz Can
2020-08-01 21:58         ` [PATCH v5] " Cengiz Can
2020-08-01 22:01         ` [PATCH v6] " Cengiz Can
2020-08-06 18:34           ` Cengiz Can
2020-08-06 18:39             ` Greg KH
2020-08-06 20:38               ` Cengiz Can
2020-08-06 22:15     ` Bjorn Helgaas [this message]
2020-08-07  9:53       ` [PATCH] " Dan Carpenter

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=20200806221537.GA703560@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=cengiz@kernel.wtf \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=sakari.ailus@linux.intel.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

DriverDev-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/driverdev-devel/0 driverdev-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 driverdev-devel driverdev-devel/ https://lore.kernel.org/driverdev-devel \
		driverdev-devel@linuxdriverproject.org devel@driverdev.osuosl.org
	public-inbox-index driverdev-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linuxdriverproject.driverdev-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git