Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46723 )
Change subject: libpayload/keyboard: Use `bool` as return type ......................................................................
libpayload/keyboard: Use `bool` as return type
Use `bool` whenever `0` was used to indicate an error. The mixing of different types for return values was mildly confusing and potentially dangerous with the i8042 API close by that uses `0` for success.
Change-Id: I876bb5076c4921f36e3438f359be8ac4c09248cc Signed-off-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/i8042/keyboard.c M payloads/libpayload/include/libpayload.h 2 files changed, 13 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/46723/1
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index eb94199..2dec3a3 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -27,6 +27,8 @@ * SUCH DAMAGE. */
+#include <stdbool.h> + #include <keycodes.h> #include <libpayload-config.h> #include <libpayload.h> @@ -169,14 +171,14 @@ #endif };
-static unsigned char keyboard_cmd(unsigned char cmd) +static bool keyboard_cmd(unsigned char cmd) { i8042_write_data(cmd);
return i8042_wait_read_ps2() == 0xfa; }
-int keyboard_havechar(void) +bool keyboard_havechar(void) { return i8042_data_ready_ps2(); } @@ -306,13 +308,13 @@ }
static struct console_input_driver cons = { - .havekey = keyboard_havechar, + .havekey = (int (*)(void))keyboard_havechar, .getchar = keyboard_getchar, .input_type = CONSOLE_INPUT_TYPE_EC, };
/* Enable keyboard translated */ -static int enable_translated(void) +static bool enable_translated(void) { if (!i8042_cmd(I8042_CMD_RD_CMD_BYTE)) { int cmd = i8042_read_data_ps2(); @@ -321,19 +323,19 @@ i8042_write_data(cmd); } else { printf("ERROR: i8042_cmd WR_CMD failed!\n"); - return 0; + return false; } } else { printf("ERROR: i8042_cmd RD_CMD failed!\n"); - return 0; + return false; } - return 1; + return true; }
/* Set scancode set 1 */ -static int set_scancode_set(void) +static bool set_scancode_set(void) { - unsigned int ret; + bool ret; ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); if (!ret) { printf("ERROR: Keyboard set scancode failed!\n"); diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index b761f04..fa501a7 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -42,6 +42,7 @@ #ifndef _LIBPAYLOAD_H #define _LIBPAYLOAD_H
+#include <stdbool.h> #include <libpayload-config.h> #include <compiler.h> #include <cbgfx.h> @@ -186,7 +187,7 @@ */ void keyboard_init(void); void keyboard_disconnect(void); -int keyboard_havechar(void); +bool keyboard_havechar(void); unsigned char keyboard_get_scancode(void); int keyboard_getchar(void); int keyboard_set_layout(char *country);
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46723 )
Change subject: libpayload/keyboard: Use `bool` as return type ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46723 )
Change subject: libpayload/keyboard: Use `bool` as return type ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/46723/1/payloads/libpayload/drivers... File payloads/libpayload/drivers/i8042/keyboard.c:
https://review.coreboot.org/c/coreboot/+/46723/1/payloads/libpayload/drivers... PS1, Line 311: (int (*)(void)) heh, didn't know one can cast function pointers
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46723 )
Change subject: libpayload/keyboard: Use `bool` as return type ......................................................................
libpayload/keyboard: Use `bool` as return type
Use `bool` whenever `0` was used to indicate an error. The mixing of different types for return values was mildly confusing and potentially dangerous with the i8042 API close by that uses `0` for success.
Change-Id: I876bb5076c4921f36e3438f359be8ac4c09248cc Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/46723 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Angel Pons th3fanbus@gmail.com --- M payloads/libpayload/drivers/i8042/keyboard.c M payloads/libpayload/include/libpayload.h 2 files changed, 13 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index eb94199..2dec3a3 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -27,6 +27,8 @@ * SUCH DAMAGE. */
+#include <stdbool.h> + #include <keycodes.h> #include <libpayload-config.h> #include <libpayload.h> @@ -169,14 +171,14 @@ #endif };
-static unsigned char keyboard_cmd(unsigned char cmd) +static bool keyboard_cmd(unsigned char cmd) { i8042_write_data(cmd);
return i8042_wait_read_ps2() == 0xfa; }
-int keyboard_havechar(void) +bool keyboard_havechar(void) { return i8042_data_ready_ps2(); } @@ -306,13 +308,13 @@ }
static struct console_input_driver cons = { - .havekey = keyboard_havechar, + .havekey = (int (*)(void))keyboard_havechar, .getchar = keyboard_getchar, .input_type = CONSOLE_INPUT_TYPE_EC, };
/* Enable keyboard translated */ -static int enable_translated(void) +static bool enable_translated(void) { if (!i8042_cmd(I8042_CMD_RD_CMD_BYTE)) { int cmd = i8042_read_data_ps2(); @@ -321,19 +323,19 @@ i8042_write_data(cmd); } else { printf("ERROR: i8042_cmd WR_CMD failed!\n"); - return 0; + return false; } } else { printf("ERROR: i8042_cmd RD_CMD failed!\n"); - return 0; + return false; } - return 1; + return true; }
/* Set scancode set 1 */ -static int set_scancode_set(void) +static bool set_scancode_set(void) { - unsigned int ret; + bool ret; ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); if (!ret) { printf("ERROR: Keyboard set scancode failed!\n"); diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index b761f04..fa501a7 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -42,6 +42,7 @@ #ifndef _LIBPAYLOAD_H #define _LIBPAYLOAD_H
+#include <stdbool.h> #include <libpayload-config.h> #include <compiler.h> #include <cbgfx.h> @@ -186,7 +187,7 @@ */ void keyboard_init(void); void keyboard_disconnect(void); -int keyboard_havechar(void); +bool keyboard_havechar(void); unsigned char keyboard_get_scancode(void); int keyboard_getchar(void); int keyboard_set_layout(char *country);
-
Angel Pons (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)