linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Grant Likely <grant.likely@secretlab.ca>
To: Andy Fleming <afleming@freescale.com>,
	netdev <netdev@vger.kernel.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: ERR_PTR pattern in phylib
Date: Wed, 1 Sep 2010 08:42:49 -0600	[thread overview]
Message-ID: <AANLkTimjVZTNeLdm4qptU1MqREODLASdGGanUyxqxfub@mail.gmail.com> (raw)

Hi Andy,

I've been looking at the phylib code, and specifically at the use of
the ERR_PTR() pattern.  I'm personally not a big fan of ERR_PTR()
because the compiler cannot type check the result and it runs
completely counter to the pattern "if (!ptr)" than is common for
testing a pointer return value.

(That being said, I do understand the need for it in certain parts of
the kernel (like the filesystem code) where it is important to be both
efficient because it is a hot path and still able to accurately return
an error code that is used by userspace.)

It seems to me that phylib is one of the cases where the users (the
network drivers) don't actually care about the specific error code
when calling phylib functions.  The drivers only seem to care whether
or not the function failed, and if it did then bail out.  I've also
noticed that using the "if (!ptr)" test on phylib return values is a
common error for driver writers.

In the interest of making driver code easier to write and review,
would you be opposed to a set of patches to remove the ERR_PTR()
pattern from phylib and its users?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

             reply	other threads:[~2010-09-01 14:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-01 14:42 Grant Likely [this message]
2010-09-01 15:27 ` ERR_PTR pattern in phylib David Miller
2010-09-01 18:56   ` Grant Likely
2010-09-01 19:03     ` David Miller
2010-09-01 15:50 ` Fleming Andy-AFLEMING
2010-09-02 15:20 ` Maciej W. Rozycki
2010-09-02 16:34   ` David Miller
2010-09-02 16:43     ` Maciej W. Rozycki

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=AANLkTimjVZTNeLdm4qptU1MqREODLASdGGanUyxqxfub@mail.gmail.com \
    --to=grant.likely@secretlab.ca \
    --cc=afleming@freescale.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).