public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] musb: sunxi: Implement dfu_usb_get_reset()
Date: Sun, 25 Oct 2015 17:08:38 +0100	[thread overview]
Message-ID: <201510251708.39701.marex@denx.de> (raw)
In-Reply-To: <20151025164615.295bf50c@i7>

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@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@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

  reply	other threads:[~2015-10-25 16:08 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-25  4:44 [U-Boot] [PATCH 0/2] Enable DFU for RAM on Allwinner devices Siarhei Siamashka
2015-10-25  4:44 ` [U-Boot] [PATCH 1/2] sunxi: Enable DFU for RAM Siarhei Siamashka
2015-11-20 15:21   ` [U-Boot] [U-Boot,1/2] " Hans de Goede
2015-10-25  4:44 ` [U-Boot] [PATCH 2/2] musb: sunxi: Implement dfu_usb_get_reset() Siarhei Siamashka
2015-10-25 11:00   ` Marek Vasut
2015-10-25 11:46     ` Albert ARIBAUD
2015-10-25 12:40       ` Ian Campbell
2015-10-25 13:22         ` Albert ARIBAUD
2015-10-25 13:29           ` Marek Vasut
2015-10-25 14:46             ` Siarhei Siamashka
2015-10-25 16:08               ` Marek Vasut [this message]
2015-10-25 19:24               ` Ian Campbell
2015-10-25 20:48                 ` Siarhei Siamashka
2015-10-25 19:22           ` Ian Campbell
2015-10-25 21:16             ` Albert ARIBAUD
2015-10-26 10:07               ` Ian Campbell
2015-10-26 11:32                 ` Albert ARIBAUD
2015-10-25 14:55     ` Siarhei Siamashka
2015-10-25 16:09       ` Marek Vasut
2015-10-25 21:19       ` Albert ARIBAUD
2015-10-26 11:18 ` [U-Boot] [PATCH 0/2] Enable DFU for RAM on Allwinner devices Piotr Król
2015-10-27  4:31   ` Siarhei Siamashka
2015-10-27 23:27     ` Piotr Król
2015-10-28  4:49       ` Siarhei Siamashka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201510251708.39701.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox