Proxmark3 community

iceman

Administrator

Registered: 2013-04-25

Posts: 9,537

Website

possible bug: LegicRfSimulate ?

I found this one inside  "LegicRfSimulate"  method,   in  legicrf.c

the double exclamation marks smells not correct.  Is there some purpuse with this double?

 int level = !!(AT91C_BASE_PIOA->PIO_PDSR & GPIO_SSC_DIN);

piwi

Contributor

Registered: 2013-06-04

Posts: 704

Re: possible bug: LegicRfSimulate ?

This is equivalent to

int level = ((AT91C_BASE_PIOA->PIO_PDSR & GPIO_SSC_DIN) == 0 ? 0 : 1)

Some may say its very fine art of programming...

iceman

Administrator

Registered: 2013-04-25

Posts: 9,537

Website

Re: possible bug: LegicRfSimulate ?

Piwi,  you taught me something new today.    I really dislike the !! since it confuses from a misspelled singel !..   

So it basically lets zero be zero and n be 1.   Why even bother with a int then?

Enio

Contributor

Registered: 2013-09-24

Posts: 175

Re: possible bug: LegicRfSimulate ?

No, in that case its just "fun" as gpio_ssc_din is a mask handing back a single bit, it can only be 1 or 0 anyways.

Last edited by Enio (2014-12-09 21:05:02)

iceman

Administrator

Registered: 2013-04-25

Posts: 9,537

Website

Re: possible bug: LegicRfSimulate ?

so we can drop both exclaimation marks then?

holiman

Contributor

Registered: 2013-05-03

Posts: 566

Re: possible bug: LegicRfSimulate ?

We could.. but... in order to veriify that "aha, the level is a boolean 1 or 0" you'd need to have pretty deep knowledge about gpio_ssc_din. As it is now, it is painfully obvious to the most casual observer that 'level' will be either 1 or 0.

...ok, it's maybe not that self-evident, but at least it makes sense to have it there in my mind. Or to use the explicit way to write it, which is more friendly but less 'elegant'... for certain values of 'elegant'....

Oh, and to answer the second question, "why even bother with an int then" - that's what we have. It is a low level language, you can dress an int up and call it boolean, but it's still an int anyway.

Last edited by holiman (2014-12-09 23:28:45)

holiman

Contributor

Registered: 2013-05-03

Posts: 566

Re: possible bug: LegicRfSimulate ?

No, in that case its just "fun" as gpio_ssc_din is a mask handing back a single bit, it can only be 1 or 0 anyways.

Actually no. It is a mask, yes, but it does not mask the least significant bit. Ergo, it won't be 1.
Abbreviated results from a quick check:

[~/workspace/pm3-master-clean]
#ack-grep GPIO_SSC_DIN
include/config_gpio.h
30:#define GPIO_SSC_DIN		AT91C_PA18_RD
[~/workspace/pm3-master-clean]
#ack-grep AT91C_PA18_RD
include/at91sam7s512.h
2163:#define AT91C_PA18_RD       (AT91C_PIO_PA18) //  SSC Receive Data
[~/workspace/pm3-master-clean]
#ack-grep AT91C_PIO_PA18
include/at91sam7s512.h
2162:#define AT91C_PIO_PA18       (1 << 18) // Pin Controlled by PA18

Enio

Contributor

Registered: 2013-09-24

Posts: 175

Re: possible bug: LegicRfSimulate ?

Well ok, let me phrase this different.

We dont get back a "bit"  in the sense of 0x......0 and 0x.....1, it might be 0x00000000000000000000000 or 0x0000100000000000000000, however since a value becomes false only when its 0, any other value > 0 will become true. I meant to point out that theres only 2 values the result can have after the & with the bitmask which map to distinct boolean value.

In that sense we can omit the !!

Last edited by Enio (2014-12-10 11:43:04)

holiman

Contributor

Registered: 2013-05-03

Posts: 566

Re: possible bug: LegicRfSimulate ?

Well, that totally depends on how 'level' is used afterwards. If we omit the !!, it will fail if it subsequently is begin compared to 1. Or if 'level' is defined as a uin8_t, so the bit (bit 18) can't fit into it. 
If the statement is used in a loop-clause, which is often the case, there is usually no !! there, since they're obviously not needed.

Edit: To conclude, I would say it serves a good purpose

Last edited by holiman (2014-12-10 11:46:51)

iceman

Administrator

Registered: 2013-04-25

Posts: 9,537

Website

Re: possible bug: LegicRfSimulate ?

yes, ok, fine,  how about we use the  ? 0 : 1  statments for clarity.  I dislike trying to figure out obscure c-language features.

Enio

Contributor

Registered: 2013-09-24

Posts: 175

Re: possible bug: LegicRfSimulate ?

Agreed, having that in an int is an issue without conversion.
Also agreed == 0 ? 0 : 1; is the most clear.