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

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

Commit Message

Colin Ian King Nov. 13, 2018, 12:46 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_log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alex Hung Nov. 13, 2018, 12:52 p.m. UTC | #1
On 2018-11-13 8:46 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_log.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index 5a7151ec..9d3b3c65 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -578,7 +578,7 @@ char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
>  	size_t len = 0;
>  
>  	for (i = 0; i < 32; i++) {
> -		fwts_log_type mask = 1 << i;
> +		fwts_log_type mask = 1UL << i;
>  		if (type & mask) {
>  			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
>  				free(filenames);
> @@ -642,7 +642,7 @@ fwts_log *fwts_log_open(
>  	 *  the logging
>  	 */
>  	for (i = 0; i < 32; i++) {
> -		fwts_log_type mask = 1 << i;	/* The log type for this iteration */
> +		fwts_log_type mask = 1UL << i;	/* The log type for this iteration */
>  
>  		/* If set then go and open up a log for this log type */
>  		if (type & mask) {
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
ivanhu Nov. 14, 2018, 2:25 a.m. UTC | #2
On 11/13/18 8:46 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_log.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
> index 5a7151ec..9d3b3c65 100644
> --- a/src/lib/src/fwts_log.c
> +++ b/src/lib/src/fwts_log.c
> @@ -578,7 +578,7 @@ char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
>  	size_t len = 0;
>  
>  	for (i = 0; i < 32; i++) {
> -		fwts_log_type mask = 1 << i;
> +		fwts_log_type mask = 1UL << i;
>  		if (type & mask) {
>  			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
>  				free(filenames);
> @@ -642,7 +642,7 @@ fwts_log *fwts_log_open(
>  	 *  the logging
>  	 */
>  	for (i = 0; i < 32; i++) {
> -		fwts_log_type mask = 1 << i;	/* The log type for this iteration */
> +		fwts_log_type mask = 1UL << i;	/* The log type for this iteration */
>  
>  		/* If set then go and open up a log for this log type */
>  		if (type & mask) {

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

Patch
diff mbox series

diff --git a/src/lib/src/fwts_log.c b/src/lib/src/fwts_log.c
index 5a7151ec..9d3b3c65 100644
--- a/src/lib/src/fwts_log.c
+++ b/src/lib/src/fwts_log.c
@@ -578,7 +578,7 @@  char *fwts_log_get_filenames(const char *filename, const fwts_log_type type)
 	size_t len = 0;
 
 	for (i = 0; i < 32; i++) {
-		fwts_log_type mask = 1 << i;
+		fwts_log_type mask = 1UL << i;
 		if (type & mask) {
 			if ((tmp = fwts_log_filename(filename, mask)) == NULL) {
 				free(filenames);
@@ -642,7 +642,7 @@  fwts_log *fwts_log_open(
 	 *  the logging
 	 */
 	for (i = 0; i < 32; i++) {
-		fwts_log_type mask = 1 << i;	/* The log type for this iteration */
+		fwts_log_type mask = 1UL << i;	/* The log type for this iteration */
 
 		/* If set then go and open up a log for this log type */
 		if (type & mask) {