netdev - Re: ERR_PTR pattern in phylib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTimkh6ytDmiCQY-gZLHpLzeux1RwMFyr=HyfcDaJ@mail.gmail.com>
Date:	Wed, 1 Sep 2010 12:56:29 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	David Miller <davem@...emloft.net>
Cc:	afleming@...escale.com, netdev@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: ERR_PTR pattern in phylib

On Wed, Sep 1, 2010 at 9:27 AM, David Miller <davem@...emloft.net> wrote:
> From: Grant Likely <grant.likely@...retlab.ca>
> Date: Wed, 1 Sep 2010 08:42:49 -0600
>
>> 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?
>
> I'm opposed to it because it means that if code actually does
> care about the error code it will no longer be able to obtain
> it.

The error codes in phylib are almost arbitrary and don't really give
enough information about where the a failure lies.  dev_err() is more
useful for debugging.

My experience has been that the encoding of error numbers into a
pointer return is a source of bugs for driver writers and should be
strongly avoided unless the return codes are actually important (ie.
userspace depends on them).  Especially so when the use-case for
keeping them is merely theoretical.  I've looked through the phylib
usage for ERR_PTR(), and it all is related to whether or not a
phy_device pointer can be located.  The error code is pretty much
irrelevant if a phy cannot be obtained.

How about this as a compromise:  I'll investigate all the users of
phylib and if I find even one situation where the specific return code
is actually important to a driver, then I'll back off.  phylib has
been around for 5 years now which should be enough time for that use
case to bubble to the surface.

g.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html