kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
* ternary vs double exclamation
@ 2014-12-30  0:25 Vinícius Tinti
  2014-12-30  0:34 ` Max Filippov
  2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
  0 siblings, 2 replies; 13+ messages in thread
From: Vinícius Tinti @ 2014-12-30  0:25 UTC (permalink / raw)
  To: kernelnewbies

Hi,

I was looking the kernel source code and there are a lot of places in
which either "(expression) ? 1 : 0" or "(expression) ? 0 : 1" appear.
As fair as I can tell both can be replaced by "!!expression" and
"!expression".

Moreover there it seems that using "!!" does not add a "nopl"
instruction at the end of the call. Does anybody knows why?

Anyway. Wouldn't be nice if kernel provides something like
"boolean(x)" macro and  "inv_boolean(x)" to do this operations?

#ifdef MOD_IF
int mod_if(int x) {
    return (x == 0) ? 0 : 1;
}
#endif

#ifdef MOD_X
int mod_x(int x) {
    return !!x;
}
#endif

0000000000000000 <mod_if>:
   0:   31 c0                   xor    %eax,%eax
   2:   85 ff                   test   %edi,%edi
   4:   0f 95 c0                setne  %al
   7:   c3                      retq
   8:   0f 1f 84 00 00 00 00    nopl   0x0(%rax,%rax,1)
   f:   00

0000000000000010 <mod_x>:
  10:   31 c0                   xor    %eax,%eax
  12:   85 ff                   test   %edi,%edi
  14:   0f 95 c0                setne  %al
  17:   c3                      retq


Regards,
Vin?cius

-- 
Simplicity is the ultimate sophistication

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2014-12-30  0:25 ternary vs double exclamation Vinícius Tinti
@ 2014-12-30  0:34 ` Max Filippov
  2014-12-30  0:40   ` Vinícius Tinti
  2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
  1 sibling, 1 reply; 13+ messages in thread
From: Max Filippov @ 2014-12-30  0:34 UTC (permalink / raw)
  To: kernelnewbies

On Tue, Dec 30, 2014 at 3:25 AM, Vin?cius Tinti <viniciustinti@gmail.com> wrote:
> I was looking the kernel source code and there are a lot of places in
> which either "(expression) ? 1 : 0" or "(expression) ? 0 : 1" appear.
> As fair as I can tell both can be replaced by "!!expression" and
> "!expression".
>
> Moreover there it seems that using "!!" does not add a "nopl"
> instruction at the end of the call. Does anybody knows why?

It seems that the nop instruction is inserted for alignment, and if you
reverse the order of functions in your c source, nop will still be
inserted between them.

> 0000000000000000 <mod_if>:
>    0:   31 c0                   xor    %eax,%eax
>    2:   85 ff                   test   %edi,%edi
>    4:   0f 95 c0                setne  %al
>    7:   c3                      retq
>    8:   0f 1f 84 00 00 00 00    nopl   0x0(%rax,%rax,1)
>    f:   00
>
> 0000000000000010 <mod_x>:
>   10:   31 c0                   xor    %eax,%eax
>   12:   85 ff                   test   %edi,%edi
>   14:   0f 95 c0                setne  %al
>   17:   c3                      retq

-- 
Thanks.
-- Max

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2014-12-30  0:34 ` Max Filippov
@ 2014-12-30  0:40   ` Vinícius Tinti
  0 siblings, 0 replies; 13+ messages in thread
From: Vinícius Tinti @ 2014-12-30  0:40 UTC (permalink / raw)
  To: kernelnewbies

On Mon, Dec 29, 2014 at 10:34 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Tue, Dec 30, 2014 at 3:25 AM, Vin?cius Tinti <viniciustinti@gmail.com> wrote:
>> I was looking the kernel source code and there are a lot of places in
>> which either "(expression) ? 1 : 0" or "(expression) ? 0 : 1" appear.
>> As fair as I can tell both can be replaced by "!!expression" and
>> "!expression".
>>
>> Moreover there it seems that using "!!" does not add a "nopl"
>> instruction at the end of the call. Does anybody knows why?
>
> It seems that the nop instruction is inserted for alignment, and if you
> reverse the order of functions in your c source, nop will still be
> inserted between them.

I notice that too. If you use both functions in other code they both will have
the nopl. And "!expression" is as good as the "(expression) ? 0 : 1".

Thus no gain and worse readability I think.

>> 0000000000000000 <mod_if>:
>>    0:   31 c0                   xor    %eax,%eax
>>    2:   85 ff                   test   %edi,%edi
>>    4:   0f 95 c0                setne  %al
>>    7:   c3                      retq
>>    8:   0f 1f 84 00 00 00 00    nopl   0x0(%rax,%rax,1)
>>    f:   00
>>
>> 0000000000000010 <mod_x>:
>>   10:   31 c0                   xor    %eax,%eax
>>   12:   85 ff                   test   %edi,%edi
>>   14:   0f 95 c0                setne  %al
>>   17:   c3                      retq
>
> --
> Thanks.
> -- Max



-- 
Simplicity is the ultimate sophistication

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2014-12-30  0:25 ternary vs double exclamation Vinícius Tinti
  2014-12-30  0:34 ` Max Filippov
@ 2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
  2014-12-30  1:04   ` Vinícius Tinti
  2015-01-03 23:54   ` John de la Garza
  1 sibling, 2 replies; 13+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2014-12-30  1:00 UTC (permalink / raw)
  To: kernelnewbies

On Mon, 29 Dec 2014 22:25:41 -0200, Vin?cius Tinti said:

> I was looking the kernel source code and there are a lot of places in
> which either "(expression) ? 1 : 0" or "(expression) ? 0 : 1" appear.
> As fair as I can tell both can be replaced by "!!expression" and
> "!expression".

As far as the compiler goes, they're the same thing, as you already discovered.

Some are just code written by not-so-experts.

In other cases, the author may have wanted to keep clear that we were
calculating an integer to be used for further arithmetic rather than a boolean.

Patches to clean it up wouldn't be a bad idea in some cases.  However, each
location will have to be examined for what was intended and what produces more
readable code.  If you're *really* ambitious, converting stuff to use a boolean
rather than an int where appropriate would be nice - we don't do that enough
currently. But that will require actually reading and understanding the code.
That's not a good task for those who like to submit patches without
thinking....

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20141229/8bd32c46/attachment.bin 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
@ 2014-12-30  1:04   ` Vinícius Tinti
  2015-01-03 23:54   ` John de la Garza
  1 sibling, 0 replies; 13+ messages in thread
From: Vinícius Tinti @ 2014-12-30  1:04 UTC (permalink / raw)
  To: kernelnewbies

On Mon, Dec 29, 2014 at 11:00 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Mon, 29 Dec 2014 22:25:41 -0200, Vin?cius Tinti said:
>
>> I was looking the kernel source code and there are a lot of places in
>> which either "(expression) ? 1 : 0" or "(expression) ? 0 : 1" appear.
>> As fair as I can tell both can be replaced by "!!expression" and
>> "!expression".
>
> As far as the compiler goes, they're the same thing, as you already discovered.
>
> Some are just code written by not-so-experts.
>
> In other cases, the author may have wanted to keep clear that we were
> calculating an integer to be used for further arithmetic rather than a boolean.
>
> Patches to clean it up wouldn't be a bad idea in some cases.  However, each
> location will have to be examined for what was intended and what produces more
> readable code.  If you're *really* ambitious, converting stuff to use a boolean
> rather than an int where appropriate would be nice - we don't do that enough
> currently. But that will require actually reading and understanding the code.
> That's not a good task for those who like to submit patches without
> thinking....
>

In fact, to be sure that I was not messing around I was thinking in
write a Clang
plugin for doing so. With this I can be 100% sure that I am not introducing any
errors.

But anyway if there is no benefits on doing so there is no need to do.

-- 
Simplicity is the ultimate sophistication

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
  2014-12-30  1:04   ` Vinícius Tinti
@ 2015-01-03 23:54   ` John de la Garza
  2015-01-04  4:20     ` Valdis.Kletnieks at vt.edu
  1 sibling, 1 reply; 13+ messages in thread
From: John de la Garza @ 2015-01-03 23:54 UTC (permalink / raw)
  To: kernelnewbies

On Mon, Dec 29, 2014 at 08:00:08PM -0500, Valdis.Kletnieks at vt.edu wrote:
> If you're *really* ambitious, converting stuff to use a boolean
> rather than an int where appropriate would be nice - we don't do that enough
> currently. But that will require actually reading and understanding the code.


It should not be assumed that true will always be 1 as defined in
include/linux/stddef.h, right?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-03 23:54   ` John de la Garza
@ 2015-01-04  4:20     ` Valdis.Kletnieks at vt.edu
  2015-01-04 23:43       ` John de la Garza
  0 siblings, 1 reply; 13+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2015-01-04  4:20 UTC (permalink / raw)
  To: kernelnewbies

On Sat, 03 Jan 2015 18:54:00 -0500, John de la Garza said:

> It should not be assumed that true will always be 1 as defined in
> include/linux/stddef.h, right?

No, I mean use an actual 'bool' type rather than 'int'.  Consider this from
kernel/softirq.c:

static inline bool lockdep_softirq_start(void)
{
        bool in_hardirq = false;

        if (trace_hardirq_context(current)) {
                in_hardirq = true;
                trace_hardirq_exit();
        }

        lockdep_softirq_enter();

        return in_hardirq;
}

However, this will require actual code analysis to make sure that it's
*really* being used as a boolean, not an int.  In particular, assignments
to/from the variable need to be audited for casting issues (and possibly
the correct rework is to convert *several* variables to bool at once).


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150103/ae4843b6/attachment.bin 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-04  4:20     ` Valdis.Kletnieks at vt.edu
@ 2015-01-04 23:43       ` John de la Garza
  2015-01-05  0:50         ` Greg KH
  2015-01-05  1:17         ` Valdis.Kletnieks at vt.edu
  0 siblings, 2 replies; 13+ messages in thread
From: John de la Garza @ 2015-01-04 23:43 UTC (permalink / raw)
  To: kernelnewbies

On Sat, Jan 03, 2015 at 11:20:29PM -0500, Valdis.Kletnieks at vt.edu wrote:
> On Sat, 03 Jan 2015 18:54:00 -0500, John de la Garza said:
> 
> > It should not be assumed that true will always be 1 as defined in
> > include/linux/stddef.h, right?
> 
> No, I mean use an actual 'bool' type rather than 'int'.  Consider this from
> kernel/softirq.c:

yes, bool has two possible values true and false

from include/linux/stddef.h:
enum {
	        false   = 0,
		true	= 1
};


I assume it is a bad idea to depend on true being 1, right?  I mean, I
should assume that true could be changed to any non 0 value in the future,
right?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-04 23:43       ` John de la Garza
@ 2015-01-05  0:50         ` Greg KH
  2015-01-08  4:46           ` John de la Garza
  2015-01-05  1:17         ` Valdis.Kletnieks at vt.edu
  1 sibling, 1 reply; 13+ messages in thread
From: Greg KH @ 2015-01-05  0:50 UTC (permalink / raw)
  To: kernelnewbies

On Sun, Jan 04, 2015 at 06:43:22PM -0500, John de la Garza wrote:
> On Sat, Jan 03, 2015 at 11:20:29PM -0500, Valdis.Kletnieks at vt.edu wrote:
> > On Sat, 03 Jan 2015 18:54:00 -0500, John de la Garza said:
> > 
> > > It should not be assumed that true will always be 1 as defined in
> > > include/linux/stddef.h, right?
> > 
> > No, I mean use an actual 'bool' type rather than 'int'.  Consider this from
> > kernel/softirq.c:
> 
> yes, bool has two possible values true and false
> 
> from include/linux/stddef.h:
> enum {
> 	        false   = 0,
> 		true	= 1
> };
> 
> 
> I assume it is a bad idea to depend on true being 1, right?  I mean, I
> should assume that true could be changed to any non 0 value in the future,
> right?

Why would that matter?  Just always test for "true" and "false" and you
will be fine.

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-04 23:43       ` John de la Garza
  2015-01-05  0:50         ` Greg KH
@ 2015-01-05  1:17         ` Valdis.Kletnieks at vt.edu
  2015-01-08  4:58           ` John de la Garza
  1 sibling, 1 reply; 13+ messages in thread
From: Valdis.Kletnieks at vt.edu @ 2015-01-05  1:17 UTC (permalink / raw)
  To: kernelnewbies

On Sun, 04 Jan 2015 18:43:22 -0500, John de la Garza said:
> On Sat, Jan 03, 2015 at 11:20:29PM -0500, Valdis.Kletnieks at vt.edu wrote:
> > On Sat, 03 Jan 2015 18:54:00 -0500, John de la Garza said:
> >
> > > It should not be assumed that true will always be 1 as defined in
> > > include/linux/stddef.h, right?
> >
> > No, I mean use an actual 'bool' type rather than 'int'.  Consider this from
> > kernel/softirq.c:
>
> yes, bool has two possible values true and false
>
> from include/linux/stddef.h:
> enum {
> 	        false   = 0,
> 		true	= 1
> };

Note that's an *anonynous* enum, which defines the two values, but
it *doesn't* define an enum type that can be used to force type safety.

No, if you're converting a variable from int to bool, the *important* line is
from include/linux/types.h:

typedef _Bool                   bool;

which ensures more type safety than the enum does.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150104/faeeeb5a/attachment.bin 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-05  0:50         ` Greg KH
@ 2015-01-08  4:46           ` John de la Garza
  2015-01-08  6:37             ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: John de la Garza @ 2015-01-08  4:46 UTC (permalink / raw)
  To: kernelnewbies

On Sun, Jan 04, 2015 at 04:50:58PM -0800, Greg KH wrote:
> On Sun, Jan 04, 2015 at 06:43:22PM -0500, John de la Garza wrote:
> > I assume it is a bad idea to depend on true being 1, right?  I mean, I
> > should assume that true could be changed to any non 0 value in the
future,
> > right?
>
> Why would that matter?  Just always test for "true" and "false" and you
> will be fine.
>

What if you need to store the value in a bitmap that needs
to be 1 or 0 so they can be shifted/anded in?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150107/f35493f4/attachment.html 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-05  1:17         ` Valdis.Kletnieks at vt.edu
@ 2015-01-08  4:58           ` John de la Garza
  0 siblings, 0 replies; 13+ messages in thread
From: John de la Garza @ 2015-01-08  4:58 UTC (permalink / raw)
  To: kernelnewbies

On Sun, Jan 04, 2015 at 08:17:15PM -0500, Valdis.Kletnieks at vt.edu wrote:
> On Sun, 04 Jan 2015 18:43:22 -0500, John de la Garza said:
> > On Sat, Jan 03, 2015 at 11:20:29PM -0500, Valdis.Kletnieks at vt.edu wrote:
> > > On Sat, 03 Jan 2015 18:54:00 -0500, John de la Garza said:
> > >
> > > > It should not be assumed that true will always be 1 as defined in
> > > > include/linux/stddef.h, right?
> > >
> > > No, I mean use an actual 'bool' type rather than 'int'.  Consider this from
> > > kernel/softirq.c:
> >
> > yes, bool has two possible values true and false
> >
> > from include/linux/stddef.h:
> > enum {
> > 	        false   = 0,
> > 		true	= 1
> > };
> 
> Note that's an *anonynous* enum, which defines the two values, but
> it *doesn't* define an enum type that can be used to force type safety.
> 
> No, if you're converting a variable from int to bool, the *important* line is
> from include/linux/types.h:
> 
> typedef _Bool                   bool;
> 
> which ensures more type safety than the enum does.

right, I see that now

so _Bool is a defined by the compiler and typedefed to bool 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* ternary vs double exclamation
  2015-01-08  4:46           ` John de la Garza
@ 2015-01-08  6:37             ` Greg KH
  0 siblings, 0 replies; 13+ messages in thread
From: Greg KH @ 2015-01-08  6:37 UTC (permalink / raw)
  To: kernelnewbies

On Wed, Jan 07, 2015 at 08:46:52PM -0800, John de la Garza wrote:
> On Sun, Jan 04, 2015 at 04:50:58PM -0800, Greg KH wrote:
> > On Sun, Jan 04, 2015 at 06:43:22PM -0500, John de la Garza wrote:
> > > I assume it is a bad idea to depend on true being 1, right?? I mean, I
> > > should assume that true could be changed to any non 0 value in the future,
> > > right?
> >
> > Why would that matter?? Just always test for "true" and "false" and you
> > will be fine.
> >
> 
> What if you need to store the value in a bitmap that needs
> to be 1 or 0 so they can be shifted/anded in?

Then you don't use a boolean if you are worried about it, come on, why
is this even a discussion?

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-01-08  6:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-30  0:25 ternary vs double exclamation Vinícius Tinti
2014-12-30  0:34 ` Max Filippov
2014-12-30  0:40   ` Vinícius Tinti
2014-12-30  1:00 ` Valdis.Kletnieks at vt.edu
2014-12-30  1:04   ` Vinícius Tinti
2015-01-03 23:54   ` John de la Garza
2015-01-04  4:20     ` Valdis.Kletnieks at vt.edu
2015-01-04 23:43       ` John de la Garza
2015-01-05  0:50         ` Greg KH
2015-01-08  4:46           ` John de la Garza
2015-01-08  6:37             ` Greg KH
2015-01-05  1:17         ` Valdis.Kletnieks at vt.edu
2015-01-08  4:58           ` John de la Garza

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).