lib: fwts_coreboot_cbmem: shift UL values rather than signed int values
diff mbox series

Message ID 20181113124354.16876-1-colin.king@canonical.com
State Accepted
Headers show
Series
  • lib: fwts_coreboot_cbmem: shift UL values rather than signed int values
Related show

Commit Message

Colin King Nov. 13, 2018, 12:43 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

Shifting signed int values can result in undefined behaviour if the shift
is 31 places.  Make all shifted values of 1 unsigned long to ensure there
is no undefined behaviour. Cleans up clang error messages such as:

error: Shifting signed 32-bit value by 31 bits is undefined behaviour

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alex Hung Nov. 13, 2018, 12:53 p.m. UTC | #1
On 2018-11-13 8:43 p.m., Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Shifting signed int values can result in undefined behaviour if the shift
> is 31 places.  Make all shifted values of 1 unsigned long to ensure there
> is no undefined behaviour. Cleans up clang error messages such as:
> 
> error: Shifting signed 32-bit value by 31 bits is undefined behaviour
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
> index e02c523c..3802a0af 100644
> --- a/src/lib/src/fwts_coreboot_cbmem.c
> +++ b/src/lib/src/fwts_coreboot_cbmem.c
> @@ -32,8 +32,8 @@
>  #include <fcntl.h>
>  
>  
> -#define CURSOR_MASK ((1 << 28) - 1)
> -#define OVERFLOW (1 << 31)
> +#define CURSOR_MASK ((1UL << 28) - 1)
> +#define OVERFLOW (1UL << 31)
>  
>  #define LB_TAG_CBMEM_CONSOLE	0x0017
>  #define LB_TAG_FORWARD		0x0011
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
ivanhu Nov. 14, 2018, 2:24 a.m. UTC | #2
On 11/13/18 8:43 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Shifting signed int values can result in undefined behaviour if the shift
> is 31 places.  Make all shifted values of 1 unsigned long to ensure there
> is no undefined behaviour. Cleans up clang error messages such as:
>
> error: Shifting signed 32-bit value by 31 bits is undefined behaviour
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
> index e02c523c..3802a0af 100644
> --- a/src/lib/src/fwts_coreboot_cbmem.c
> +++ b/src/lib/src/fwts_coreboot_cbmem.c
> @@ -32,8 +32,8 @@
>  #include <fcntl.h>
>  
>  
> -#define CURSOR_MASK ((1 << 28) - 1)
> -#define OVERFLOW (1 << 31)
> +#define CURSOR_MASK ((1UL << 28) - 1)
> +#define OVERFLOW (1UL << 31)
>  
>  #define LB_TAG_CBMEM_CONSOLE	0x0017
>  #define LB_TAG_FORWARD		0x0011


Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch
diff mbox series

diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
index e02c523c..3802a0af 100644
--- a/src/lib/src/fwts_coreboot_cbmem.c
+++ b/src/lib/src/fwts_coreboot_cbmem.c
@@ -32,8 +32,8 @@ 
 #include <fcntl.h>
 
 
-#define CURSOR_MASK ((1 << 28) - 1)
-#define OVERFLOW (1 << 31)
+#define CURSOR_MASK ((1UL << 28) - 1)
+#define OVERFLOW (1UL << 31)
 
 #define LB_TAG_CBMEM_CONSOLE	0x0017
 #define LB_TAG_FORWARD		0x0011