Re: [PATCH] regulator: Fix and simplify debugfs support

[Stephen Boyd]

On 2/19/2012 6:12 PM, Mark Brown wrote:
> On Sat, Feb 18, 2012 at 11:28:25PM -0800, Stephen Boyd wrote:
> > If CONFIG_DEBUG_FS=y debugfs functions will never return an
> > ERR_PTR. Instead they'll return NULL. The intent is to remove
> > ifdefs in calling code.
> > Instead of checking for an ERR_PTR check for NULL. This simplifies
> > the code and also fixes an error check that would never have
> > worked otherwise. While we're here modernize the code to use
> > S_IRUGO instead of 0444.
> This was actually a deliberate decision to make the code more robust
> against change - the IS_ERR_OR_NULL doesn't make the code any bigger but
> it means it's less likely to break in the face of changes.

More robust? The debugfs code in the regulator core looks confused on 
what the return value is. Sometimes it's IS_ERR, sometimes it's NULL, 
sometimes it's both. Might as well clean it up to be consistent and proper.

> > If we're willing to sacrifice a pointer per rdev we can remove the
> > ifdefs and the compiler should be able to optimize away the dead
> > code.
> Personally I'd be happy to do that, the only reason I put the ifdefs in
> there was that it appears to be idiomatic to do so but I'm not really a
> big fan of it.  Then again I never build kernls without debugfs support
> in them myself...

Ok. I'll send a patch to do that too.

How about a two part series with the erroneous error check fix in one 
and then the other stuff that isn't critical in another?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/