On Mon, 9 May 2011 22:36:12 +0200 Mark Brown <[email protected]> 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 <stdio.h>
/* 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;
}
pgpBdB6cxsb7X.pgp
Description: PGP signature
