Patchwork hpet: hpetcheck: fix a bug found by smatch

login
register
mail settings
Submitter Colin King
Date Jan. 16, 2013, 12:53 a.m.
Message ID <1358297630-7279-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/212358/
State Accepted
Headers show

Comments

Colin King - Jan. 16, 2013, 12:53 a.m.
From: Colin Ian King <colin.king@canonical.com>

smatch found a bug that needs fixing:

"hpet_check.c:88 hpet_parse_device_hpet() error:
we previously assumed 'tmp_item' could be null (see line 81)"

Re-work the HPET parsing so that we sanely handle tmp_item.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/hpet/hpet_check/hpet_check.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)
Keng-Yu Lin - Jan. 29, 2013, 7:28 a.m.
On Wed, Jan 16, 2013 at 8:53 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> smatch found a bug that needs fixing:
>
> "hpet_check.c:88 hpet_parse_device_hpet() error:
> we previously assumed 'tmp_item' could be null (see line 81)"
>
> Re-work the HPET parsing so that we sanely handle tmp_item.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/hpet/hpet_check/hpet_check.c | 32 ++++++++++++++------------------
>  1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/src/hpet/hpet_check/hpet_check.c b/src/hpet/hpet_check/hpet_check.c
> index 7709f88..8fe124a 100644
> --- a/src/hpet/hpet_check/hpet_check.c
> +++ b/src/hpet/hpet_check/hpet_check.c
> @@ -70,29 +70,25 @@ static void hpet_parse_device_hpet(fwts_framework *fw,
>         const char *table, fwts_list_link *item)
>  {
>         for (;item != NULL; item = item->next) {
> -               char *str = fwts_text_list_text(item);
> +               const char *str = fwts_text_list_text(item);
> +
>                 if ((strstr(str, "Name") != NULL) &&
> -                       (strstr(str, "ResourceTemplate") != NULL)) {
> +                   (strstr(str, "ResourceTemplate") != NULL)) {
>                         fwts_list_link *tmp_item = item->next;
>                         for (; tmp_item != NULL; tmp_item = tmp_item->next) {
> -                               if (strstr(fwts_text_list_text(tmp_item),
> -                                       "Memory32Fixed") != NULL) {
> -                                       tmp_item = tmp_item->next;
> -                                       if (tmp_item != NULL) {
> -                                               hpet_parse_check_base(fw, table, tmp_item);
> +                               const char *str = fwts_text_list_text(tmp_item);
> +
> +                               if (strstr(str, "Memory32Fixed") != NULL) {
> +                                       /* Next line contains base address */
> +                                       if (tmp_item->next != NULL) {
> +                                               hpet_parse_check_base(fw, table, tmp_item->next);
>                                                 return;
>                                         }
> -                               }
> -                               if (strstr(fwts_text_list_text(tmp_item),
> -                                       "DWordMemory") != NULL) {
> -                                       tmp_item = tmp_item->next;
> -                                       if (tmp_item != NULL) {
> -                                               tmp_item = tmp_item->next;
> -                                               if (tmp_item != NULL) {
> -                                                       /* HPET section is found, get base */
> -                                                       hpet_parse_check_base(fw, table, tmp_item);
> -                                                       return;
> -                                               }
> +                               } else if (strstr(str, "DWordMemory") != NULL) {
> +                                       if (tmp_item->next != NULL &&           /* Granularity */
> +                                           tmp_item->next->next != NULL) {     /* Base address */
> +                                               hpet_parse_check_base(fw, table, tmp_item->next->next);
> +                                               return;
>                                         }
>                                 }
>                         }
> --
> 1.8.0
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Alex Hung - Jan. 31, 2013, 7:33 a.m.
On 01/16/2013 08:53 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> smatch found a bug that needs fixing:
>
> "hpet_check.c:88 hpet_parse_device_hpet() error:
> we previously assumed 'tmp_item' could be null (see line 81)"
>
> Re-work the HPET parsing so that we sanely handle tmp_item.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/hpet/hpet_check/hpet_check.c | 32 ++++++++++++++------------------
>   1 file changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/src/hpet/hpet_check/hpet_check.c b/src/hpet/hpet_check/hpet_check.c
> index 7709f88..8fe124a 100644
> --- a/src/hpet/hpet_check/hpet_check.c
> +++ b/src/hpet/hpet_check/hpet_check.c
> @@ -70,29 +70,25 @@ static void hpet_parse_device_hpet(fwts_framework *fw,
>   	const char *table, fwts_list_link *item)
>   {
>   	for (;item != NULL; item = item->next) {
> -		char *str = fwts_text_list_text(item);
> +		const char *str = fwts_text_list_text(item);
> +
>   		if ((strstr(str, "Name") != NULL) &&
> -			(strstr(str, "ResourceTemplate") != NULL)) {
> +		    (strstr(str, "ResourceTemplate") != NULL)) {
>   			fwts_list_link *tmp_item = item->next;
>   			for (; tmp_item != NULL; tmp_item = tmp_item->next) {
> -				if (strstr(fwts_text_list_text(tmp_item),
> -					"Memory32Fixed") != NULL) {
> -					tmp_item = tmp_item->next;
> -					if (tmp_item != NULL) {
> -						hpet_parse_check_base(fw, table, tmp_item);
> +				const char *str = fwts_text_list_text(tmp_item);
> +
> +				if (strstr(str, "Memory32Fixed") != NULL) {
> +					/* Next line contains base address */
> +					if (tmp_item->next != NULL) {
> +						hpet_parse_check_base(fw, table, tmp_item->next);
>   						return;
>   					}
> -				}
> -				if (strstr(fwts_text_list_text(tmp_item),
> -					"DWordMemory") != NULL) {
> -					tmp_item = tmp_item->next;
> -					if (tmp_item != NULL) {
> -						tmp_item = tmp_item->next;
> -						if (tmp_item != NULL) {
> -							/* HPET section is found, get base */
> -							hpet_parse_check_base(fw, table, tmp_item);
> -							return;
> -						}
> +				} else if (strstr(str, "DWordMemory") != NULL) {
> +					if (tmp_item->next != NULL &&		/* Granularity */
> +					    tmp_item->next->next != NULL) {	/* Base address */
> +						hpet_parse_check_base(fw, table, tmp_item->next->next);
> +						return;
>   					}
>   				}
>   			}
>
Acked-by: Alex Hung <alex.hung@canonical.com>

Patch

diff --git a/src/hpet/hpet_check/hpet_check.c b/src/hpet/hpet_check/hpet_check.c
index 7709f88..8fe124a 100644
--- a/src/hpet/hpet_check/hpet_check.c
+++ b/src/hpet/hpet_check/hpet_check.c
@@ -70,29 +70,25 @@  static void hpet_parse_device_hpet(fwts_framework *fw,
 	const char *table, fwts_list_link *item)
 {
 	for (;item != NULL; item = item->next) {
-		char *str = fwts_text_list_text(item);
+		const char *str = fwts_text_list_text(item);
+
 		if ((strstr(str, "Name") != NULL) &&
-			(strstr(str, "ResourceTemplate") != NULL)) {
+		    (strstr(str, "ResourceTemplate") != NULL)) {
 			fwts_list_link *tmp_item = item->next;
 			for (; tmp_item != NULL; tmp_item = tmp_item->next) {
-				if (strstr(fwts_text_list_text(tmp_item),
-					"Memory32Fixed") != NULL) {
-					tmp_item = tmp_item->next;
-					if (tmp_item != NULL) {
-						hpet_parse_check_base(fw, table, tmp_item);
+				const char *str = fwts_text_list_text(tmp_item);
+
+				if (strstr(str, "Memory32Fixed") != NULL) {
+					/* Next line contains base address */
+					if (tmp_item->next != NULL) {
+						hpet_parse_check_base(fw, table, tmp_item->next);
 						return;
 					}
-				}
-				if (strstr(fwts_text_list_text(tmp_item),
-					"DWordMemory") != NULL) {
-					tmp_item = tmp_item->next;
-					if (tmp_item != NULL) {
-						tmp_item = tmp_item->next;
-						if (tmp_item != NULL) {
-							/* HPET section is found, get base */
-							hpet_parse_check_base(fw, table, tmp_item);
-							return;
-						}
+				} else if (strstr(str, "DWordMemory") != NULL) {
+					if (tmp_item->next != NULL &&		/* Granularity */
+					    tmp_item->next->next != NULL) {	/* Base address */
+						hpet_parse_check_base(fw, table, tmp_item->next->next);
+						return;
 					}
 				}
 			}