[U-Boot] [PATCH 2/2] musb: sunxi: Implement dfu_usb_get_reset()

[Marek Vasut]

On Sunday, October 25, 2015 at 03:46:15 PM, Siarhei Siamashka wrote:
> On Sun, 25 Oct 2015 14:29:59 +0100
> 
> Marek Vasut <marex at denx.de> wrote:
> > On Sunday, October 25, 2015 at 02:22:53 PM, Albert ARIBAUD wrote:
> > > Hello Ian,
> > 
> > Hi!
> > 
> > > On Sun, 25 Oct 2015 12:40:45 +0000, Ian Campbell
> > > 
> > > <ijc+uboot at hellion.org.uk> wrote:
> > > > On Sun, 2015-10-25 at 12:46 +0100, Albert ARIBAUD wrote:
> > > > > > > +static u8 last_int_usb;
> > > > > > > +
> > > > > > > +bool dfu_usb_get_reset(void)
> > > > > > > +{
> > > > > > > +	return !!(last_int_usb & MUSB_INTR_RESET);
> > > > > > 
> > > > > > The !! is not needed.
> > > > > 
> > > > > Except if you want to be sure that you return 0 or 1 rather than 0
> > > > > or (1 << something).
> > > > 
> > > > Doesn't the bool return type already cause that to happen? (from the
> > > > PoV of the caller at least)
> > > 
> > > When all is said and done, a C bool is a C int, and anyway C does not
> > > perform value conversion (except for size and possibly sign extension)
> > > on type casts.
> > > 
> > > So no, types, bool or otherwise, do not cause any implicit '!!' to
> > > happen.
> > > 
> > > What happens is, wherever C expects a boolean value ('if', 'while'...)
> > > it considers 0 to be false and anything else to be true. But that's
> > > independent of the value's alleged type.
> > 
> > Which is the case here -- one is not supposed to test boolean type for
> > any particular value.
> 
> Sure, this works fine as long as everyone has exactly the same idea
> about how this is supposed to work. Please consider the following code:
> 
>     if (one_boolean_variable != another_boolean_variable) {
>         /* Sanity check failed, features X and Y must be either
>            both enabled or both disabled at the same time */
>     }
> 
> The author of this hypothetical code may claim that a boolean
> variable must be always 0 or 1.

This assumption is wrong.

> And both of you will have a long
> and entertaining discussion as a result.
> 
> One more example:
> 
>     #include <stdbool.h>
>     #include <stdio.h>
> 
>     bool foo(void)
>     {
>         return 123;

This is bloody confusing.

>     }
> 
>     int main(void)
>     {
>         printf("%d\n", (int)foo());

This is wrong -- the cast is outright incorrect.

>         return 0;
>     }
> 
> Guess what is printed after compiling and executing this code? Then
> replace "#include <stdbool.h>" with "typedef int bool;" and try it
> again. With the GCC compiler, the former prints "1" and the latter
> prints "123".

The code is broken, so the result is undefined.

> This stuff is a potential source of non-obvious bugs. Using "!!" is
> always safe, but may be in many cases redundant.

I'd expect that using !! will generate additional code and that's done
for no reason at all.

Best regards,
Marek Vasut