All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Ozgur <ozgurk@ieee.org>
Cc: Colin Ian King <colin.i.king@gmail.com>,
	Martin Schiller <ms@dev.tdt.de>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-x25@vger.kernel.org, netdev@vger.kernel.org,
	kernel-janitors@vger.kernel.org,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] x25: remove redundant pointer dev
Date: Mon, 9 May 2022 15:44:27 +0300	[thread overview]
Message-ID: <20220509124427.GG4009@kadam> (raw)
In-Reply-To: <CAADfD8wApw_v+uDTijY1K89WRJ_f7tkHmz=6LR086yMjEU4mWQ@mail.gmail.com>

On Mon, May 09, 2022 at 04:57:40AM +0400, Ozgur wrote:
> On Mon, May 9, 2022 at 1:45 AM Colin Ian King <colin.i.king@gmail.com> wrote:
> >
> > Pointer dev is being assigned a value that is never used, the assignment
> > and the variable are redundant and can be removed. Also replace null check
> > with the preferred !ptr idiom.
> >
> 
> Hello,
> 
> *dev pointer is device assign global linked list and shouldnt be
> touched by the driver so *dev wont get any value right?

Why are you talking about "*dev" instead of "dev"?

> Also seems to use this while network interface is initializing because
> some activation information and stats information is also kept here,
> for example, open *dev will call when ifconfig is called from.
> 
> route, link, forward these inital activate and move all values with
> net_device *dev?

It's not clear what you are saying...

When I review these kinds of patches I ask:
1) Does Colin's patch change run time behavior?  Obviosly not.
2) Is the current code buggy?  Sometimes when there is a static checker
   warning it indicates a typo in the code.  I do not see a bug in the
   original code before Colin's patch.
3) What was the author's original intent?  This code predates git but
   I think the "dev" was just a going to be a shorter name to type than
   "x25->neighbour->dev".

I honestly have no idea what you are saying.  At first I thought you
might be saying that this is stub code.  But that seems wrong.  Also we
do not allow stub code in the kernel.

regards,
dan carpenter


  reply	other threads:[~2022-05-09 12:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-08 21:45 [PATCH] x25: remove redundant pointer dev Colin Ian King
2022-05-09  0:57 ` Ozgur
2022-05-09 12:44   ` Dan Carpenter [this message]
2022-05-09 16:48     ` Ozgur
2022-05-10 10:10 ` patchwork-bot+netdevbpf

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=20220509124427.GG4009@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=colin.i.king@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-x25@vger.kernel.org \
    --cc=ms@dev.tdt.de \
    --cc=netdev@vger.kernel.org \
    --cc=ozgurk@ieee.org \
    --cc=pabeni@redhat.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.