Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	"open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Cengiz Can <cengiz@kernel.wtf>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	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

  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

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/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 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


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