Patchwork dmi: dmi_decode: DMI not used by kernel should be low failures (LP: #1148703)

login
register
mail settings
Submitter Colin King
Date March 7, 2013, 11:05 a.m.
Message ID <1362654307-7361-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/225812/
State Accepted
Headers show

Comments

Colin King - March 7, 2013, 11:05 a.m.
From: Colin Ian King <colin.king@canonical.com>

The dmi_decode test should report DMI failures as low if the strings are
not being used by the kernel and therefore not causing any real system issues.
This fixes bug LP: #11148703

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/dmi/dmi_decode/dmi_decode.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)
Keng-Yu Lin - March 8, 2013, 8:35 a.m.
On Thu, Mar 7, 2013 at 7:05 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The dmi_decode test should report DMI failures as low if the strings are
> not being used by the kernel and therefore not causing any real system issues.
> This fixes bug LP: #11148703
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/dmi/dmi_decode/dmi_decode.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c
> index e80daaa..15b18b2 100644
> --- a/src/dmi/dmi_decode/dmi_decode.c
> +++ b/src/dmi/dmi_decode/dmi_decode.c
> @@ -299,6 +299,7 @@ static void dmi_str_check_index(fwts_framework *fw,
>  {
>         char    *data = (char *)hdr->data;
>         uint8_t i = index;
> +       bool used_by_kernel = dmi_used_by_kernel(hdr->type, offset);
>
>         if (i > 0) {
>                 int     j;
> @@ -309,15 +310,18 @@ static void dmi_str_check_index(fwts_framework *fw,
>                         data += strlen(data) + 1;
>                         i--;
>                 }
> +
>                 /* Sanity checks */
>                 if (*data == '\0') {
> +                       int level = used_by_kernel ? LOG_LEVEL_HIGH : LOG_LEVEL_LOW;
> +
>                         /* This entry is clearly broken so flag it as a corrupt entry */
> -                       fwts_failed(fw, LOG_LEVEL_HIGH, DMI_STRING_INDEX_OUT_OF_RANGE,
> +                       fwts_failed(fw, level, DMI_STRING_INDEX_OUT_OF_RANGE,
>                                 "Out of range string index 0x%2.2" PRIx8
>                                 " while accessing entry '%s' "
>                                 "@ 0x%8.8" PRIx32 ", field '%s', offset 0x%2.2" PRIx8,
>                                 index, table, addr, field, offset);
> -                       if (dmi_used_by_kernel(hdr->type, offset))
> +                       if (used_by_kernel)
>                                 fwts_advice(fw,
>                                         "DMI strings are stored in a manner that uses a special "
>                                         "index to fetch the Nth string from the data. For this "
> @@ -351,14 +355,17 @@ static void dmi_str_check_index(fwts_framework *fw,
>                         }
>                 }
>                 if (failed != -1) {
> -                       if (dmi_used_by_kernel(hdr->type, offset)) {
> -                               fwts_failed(fw, LOG_LEVEL_MEDIUM, dmi_patterns[j].label,
> -                                       "String index 0x%2.2" PRIx8
> -                                       " in table entry '%s' @ 0x%8.8" PRIx32
> -                                       ", field '%s', offset 0x%2.2" PRIx8
> -                                       " has a default value '%s' and probably has "
> -                                       "not been updated by the BIOS vendor.",
> -                                       index, table, addr, field, offset, data);
> +                       int level = used_by_kernel ? LOG_LEVEL_MEDIUM : LOG_LEVEL_LOW;
> +
> +                       fwts_failed(fw, level, dmi_patterns[j].label,
> +                               "String index 0x%2.2" PRIx8
> +                               " in table entry '%s' @ 0x%8.8" PRIx32
> +                               ", field '%s', offset 0x%2.2" PRIx8
> +                               " has a default value '%s' and probably has "
> +                               "not been updated by the BIOS vendor.",
> +                               index, table, addr, field, offset, data);
> +
> +                       if (used_by_kernel) {
>                                 fwts_advice(fw,
>                                         "The DMI table contains data which is clearly been "
>                                         "left in a default setting and not been configured "
> @@ -370,13 +377,6 @@ static void dmi_str_check_index(fwts_framework *fw,
>                                         "is using sane values.");
>                         } else {
>                                 /* This string is broken, but we don't care about it too much */
> -                               fwts_failed(fw, LOG_LEVEL_LOW, dmi_patterns[j].label,
> -                                       "String index 0x%2.2" PRIx8
> -                                       " in table entry '%s' @ 0x%8.8" PRIx32
> -                                       ", field '%s', offset 0x%2.2" PRIx8
> -                                       " has a default value '%s' and probably has "
> -                                       "not been updated by the BIOS vendor.",
> -                                       index, table, addr, field, offset, data);
>                                 fwts_advice(fw,
>                                         "The DMI table contains data which is clearly been "
>                                         "left in a default setting and not been configured "
> --
> 1.8.1.2
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Ivan Hu - March 8, 2013, 9:19 a.m.
On 03/07/2013 07:05 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The dmi_decode test should report DMI failures as low if the strings are
> not being used by the kernel and therefore not causing any real system issues.
> This fixes bug LP: #11148703
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/dmi/dmi_decode/dmi_decode.c | 34 +++++++++++++++++-----------------
>   1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c
> index e80daaa..15b18b2 100644
> --- a/src/dmi/dmi_decode/dmi_decode.c
> +++ b/src/dmi/dmi_decode/dmi_decode.c
> @@ -299,6 +299,7 @@ static void dmi_str_check_index(fwts_framework *fw,
>   {
>   	char 	*data = (char *)hdr->data;
>   	uint8_t	i = index;
> +	bool used_by_kernel = dmi_used_by_kernel(hdr->type, offset);
>
>   	if (i > 0) {
>   		int 	j;
> @@ -309,15 +310,18 @@ static void dmi_str_check_index(fwts_framework *fw,
>   			data += strlen(data) + 1;
>   			i--;
>   		}
> +
>   		/* Sanity checks */
>   		if (*data == '\0') {
> +			int level = used_by_kernel ? LOG_LEVEL_HIGH : LOG_LEVEL_LOW;
> +
>   			/* This entry is clearly broken so flag it as a corrupt entry */
> -			fwts_failed(fw, LOG_LEVEL_HIGH, DMI_STRING_INDEX_OUT_OF_RANGE,
> +			fwts_failed(fw, level, DMI_STRING_INDEX_OUT_OF_RANGE,
>   				"Out of range string index 0x%2.2" PRIx8
>   				" while accessing entry '%s' "
>   				"@ 0x%8.8" PRIx32 ", field '%s', offset 0x%2.2" PRIx8,
>   				index, table, addr, field, offset);
> -			if (dmi_used_by_kernel(hdr->type, offset))
> +			if (used_by_kernel)
>   				fwts_advice(fw,
>   					"DMI strings are stored in a manner that uses a special "
>   					"index to fetch the Nth string from the data. For this "
> @@ -351,14 +355,17 @@ static void dmi_str_check_index(fwts_framework *fw,
>   			}
>   		}
>   		if (failed != -1) {
> -			if (dmi_used_by_kernel(hdr->type, offset)) {
> -				fwts_failed(fw, LOG_LEVEL_MEDIUM, dmi_patterns[j].label,
> -					"String index 0x%2.2" PRIx8
> -					" in table entry '%s' @ 0x%8.8" PRIx32
> -					", field '%s', offset 0x%2.2" PRIx8
> -					" has a default value '%s' and probably has "
> -					"not been updated by the BIOS vendor.",
> -					index, table, addr, field, offset, data);
> +			int level = used_by_kernel ? LOG_LEVEL_MEDIUM : LOG_LEVEL_LOW;
> +
> +			fwts_failed(fw, level, dmi_patterns[j].label,
> +				"String index 0x%2.2" PRIx8
> +				" in table entry '%s' @ 0x%8.8" PRIx32
> +				", field '%s', offset 0x%2.2" PRIx8
> +				" has a default value '%s' and probably has "
> +				"not been updated by the BIOS vendor.",
> +				index, table, addr, field, offset, data);
> +
> +			if (used_by_kernel) {
>   				fwts_advice(fw,
>   					"The DMI table contains data which is clearly been "
>   					"left in a default setting and not been configured "
> @@ -370,13 +377,6 @@ static void dmi_str_check_index(fwts_framework *fw,
>   					"is using sane values.");
>   			} else {
>   				/* This string is broken, but we don't care about it too much */
> -				fwts_failed(fw, LOG_LEVEL_LOW, dmi_patterns[j].label,
> -					"String index 0x%2.2" PRIx8
> -					" in table entry '%s' @ 0x%8.8" PRIx32
> -					", field '%s', offset 0x%2.2" PRIx8
> -					" has a default value '%s' and probably has "
> -					"not been updated by the BIOS vendor.",
> -					index, table, addr, field, offset, data);
>   				fwts_advice(fw,
>   					"The DMI table contains data which is clearly been "
>   					"left in a default setting and not been configured "
>
Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/dmi/dmi_decode/dmi_decode.c b/src/dmi/dmi_decode/dmi_decode.c
index e80daaa..15b18b2 100644
--- a/src/dmi/dmi_decode/dmi_decode.c
+++ b/src/dmi/dmi_decode/dmi_decode.c
@@ -299,6 +299,7 @@  static void dmi_str_check_index(fwts_framework *fw,
 {
 	char 	*data = (char *)hdr->data;
 	uint8_t	i = index;
+	bool used_by_kernel = dmi_used_by_kernel(hdr->type, offset);
 
 	if (i > 0) {
 		int 	j;
@@ -309,15 +310,18 @@  static void dmi_str_check_index(fwts_framework *fw,
 			data += strlen(data) + 1;
 			i--;
 		}
+
 		/* Sanity checks */
 		if (*data == '\0') {
+			int level = used_by_kernel ? LOG_LEVEL_HIGH : LOG_LEVEL_LOW;
+
 			/* This entry is clearly broken so flag it as a corrupt entry */
-			fwts_failed(fw, LOG_LEVEL_HIGH, DMI_STRING_INDEX_OUT_OF_RANGE,
+			fwts_failed(fw, level, DMI_STRING_INDEX_OUT_OF_RANGE,
 				"Out of range string index 0x%2.2" PRIx8
 				" while accessing entry '%s' "
 				"@ 0x%8.8" PRIx32 ", field '%s', offset 0x%2.2" PRIx8,
 				index, table, addr, field, offset);
-			if (dmi_used_by_kernel(hdr->type, offset))
+			if (used_by_kernel) 
 				fwts_advice(fw,
 					"DMI strings are stored in a manner that uses a special "
 					"index to fetch the Nth string from the data. For this "
@@ -351,14 +355,17 @@  static void dmi_str_check_index(fwts_framework *fw,
 			}
 		}
 		if (failed != -1) {
-			if (dmi_used_by_kernel(hdr->type, offset)) {
-				fwts_failed(fw, LOG_LEVEL_MEDIUM, dmi_patterns[j].label,
-					"String index 0x%2.2" PRIx8
-					" in table entry '%s' @ 0x%8.8" PRIx32
-					", field '%s', offset 0x%2.2" PRIx8
-					" has a default value '%s' and probably has "
-					"not been updated by the BIOS vendor.",
-					index, table, addr, field, offset, data);
+			int level = used_by_kernel ? LOG_LEVEL_MEDIUM : LOG_LEVEL_LOW;
+
+			fwts_failed(fw, level, dmi_patterns[j].label,
+				"String index 0x%2.2" PRIx8
+				" in table entry '%s' @ 0x%8.8" PRIx32
+				", field '%s', offset 0x%2.2" PRIx8
+				" has a default value '%s' and probably has "
+				"not been updated by the BIOS vendor.",
+				index, table, addr, field, offset, data);
+
+			if (used_by_kernel) {
 				fwts_advice(fw,
 					"The DMI table contains data which is clearly been "
 					"left in a default setting and not been configured "
@@ -370,13 +377,6 @@  static void dmi_str_check_index(fwts_framework *fw,
 					"is using sane values.");
 			} else {
 				/* This string is broken, but we don't care about it too much */
-				fwts_failed(fw, LOG_LEVEL_LOW, dmi_patterns[j].label,
-					"String index 0x%2.2" PRIx8
-					" in table entry '%s' @ 0x%8.8" PRIx32
-					", field '%s', offset 0x%2.2" PRIx8
-					" has a default value '%s' and probably has "
-					"not been updated by the BIOS vendor.",
-					index, table, addr, field, offset, data);
 				fwts_advice(fw,
 					"The DMI table contains data which is clearly been "
 					"left in a default setting and not been configured "