On Mon, 9 May 2011 22:36:12 +0200
Mark Brown wrote:
> On Mon, May 09, 2011 at 09:23:25PM +0100, Russell King - ARM Linux wrote:
>
> > NAK. This has been proposed before, and rejected. See the comments
> > against the original proposal:
>
> > http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6525/1
>
> Hrm, this looks like the bodge we're doing for !REGULATOR isn't actually
> helping here unless we do a NULL || IS_ERR() in the error check. The
> ifdefs are certainly fail.
>
[Adding Linus Walleij to CC as he was the author of a similar NAKed
patch]
I was blindly trusting code already in mainline again, and for that I
apologize, I finally took the time to look at the implementation
of IS_ERR() and test its use, and being IS_ERR(NULL) true it is not what
we want indeed, see the attached test program.
So, I am going to remove the ifdefs anyway but use IS_ERR_OR_NULL();
how does that sound? Am I still missing anything?
Or changing the regulator_get() stub to return an error (-ENOSYS?) might
be worth it?
Thanks Russel for pointing out the issue BTW.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
#include
/* from /include/linux/compiler.h */
#define likely_notrace(x) __builtin_expect(!!(x), 1)
#define unlikely_notrace(x) __builtin_expect(!!(x), 0)
#define likely(x) likely_notrace(x)
#define unlikely(x) unlikely_notrace(x)
/* from include/linux/compiler-gcc4.h */
#define __must_check__attribute__((warn_unused_result))
/* From include/linux/err.h */
#define MAX_ERRNO 4095
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
static inline long __must_check IS_ERR(const void *ptr)
{
return IS_ERR_VALUE((unsigned long)ptr);
}
static inline long __must_check IS_ERR_OR_NULL(const void *ptr)
{
return !ptr || IS_ERR_VALUE((unsigned long)ptr);
}
/* Mimic regulator_get() when CONFIG_REGULATOR is defined */
unsigned long *regulator_get_full()
{
unsigned long *ptr;
return ptr;
}
/* Mimic regulator_get() when CONFIG_REGULATOR is NOT defined */
unsigned long *regulator_get_stub()
{
return NULL; /* or maybe return -ENOSYS; ? */
}
int main(void)
{
unsigned long *vcc = NULL;
vcc = regulator_get_full();
if (IS_ERR(vcc))
printf("full: IS_ERR = true\n");
else
printf("full: IS_ERR = false\n");
if (IS_ERR_OR_NULL(vcc))
printf("full: IS_ERR_OR_NULL = true\n");
else
printf("full: IS_ERR_OR_NULL = false\n");
vcc = regulator_get_stub();
if (IS_ERR(vcc))
printf("stub: IS_ERR = true\n");
else
printf("stub: IS_ERR = false\n");
if (IS_ERR_OR_NULL(vcc))
printf("stub: IS_ERR_OR_NULL = true\n");
else
printf("stub: IS_ERR_OR_NULL = false\n");
return 0;
}
pgp1a5G2UOPfs.pgp
Description: PGP signature
