New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735561 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

ec: Consider adding bool typedef

Project Member Reported by sha...@chromium.org, Jun 21 2017

Issue description

Current ec coding standard is to use 'int' for booleans and expect that the compiler optimizes well (eg. not using 32 bits of storage). https://chromium-review.googlesource.com/#/c/540783/ has good evidence that we can't rely on such optimization.

 struct power_seq_op {
        /* enum gpio_signal in 16 bits */
        uint16_t signal;
-       uint8_t level;
+       int level;
        /* Number of milliseconds to delay after setting signal to level */
        uint8_t delay;
 };

(Note that 'struct power_seq_op' is only used with 'const' modifier, this should be optimizable).

with 'uint8_t' type:

0x0000000000019a88                __hey_flash_used = ((LOADADDR (.data) + SIZEOF (.data)) - 0x100a8000)


with 'int' type:

0x0000000000019bc8                __hey_flash_used = ((LOADADDR (.data) + SIZEOF (.data)) - 0x100a8000)



I propose adding a bool typedef for such variables and using it throughout the code base.

typedef uint8_t bool;

Any objections or comments?
 

Comment 1 by vpalatin@google.com, Jun 21 2017

A regular toolchain definition looks more like this:
#ifndef _STDBOOL_H
#define _STDBOOL_H

#define bool    _Bool
#define true    1
#define false   0

typedef _Bool Bool;
#define no (_Bool)0
#define yes (_Bool)1

typedef enum
{
   no,
   yes
} Bool;

#endif /* _STDBOOL_H */

- should still be the size of a byte (since we use short enums)
- you can rely a bit on the compiler to avoid bits vs boolean error.
- this also meets ISO C standard: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdbool.h.html

Our toolchain which is already compatible with C99 (even though I think we don't turn on the flag) already has the _Bool definition.

so we can do:
typedef _Bool bool;
#define true (_Bool)1
#define false (_Bool)0

Sign in to add a comment