Isaac Christensen (isaac.christensen@se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6996
-gerrit
commit 79add71c033786ed4f7e695d64db11d6a291cf93 Author: Hung-Te Lin hungte@chromium.org Date: Thu Apr 3 19:21:03 2014 +0800
edid: Fix string extraction in Monitor Descriptors.
The ASCII Data String in EDID Monitor Descriptor (3.10.3) is "Stored as ASCII, code page #437" and may contain special characters like '-'. The isalnum check should be removed.
Also, the "Monitor Name" (0xfc) does not need to always end with 0Ah, so the name_descriptor_terminated should be replaced by has_valid_string_termination.
Change-Id: I12a670237e12577fc971c0fbd9b2a61c82040ad3 Signed-off-by: Hung-Te Lin hungte@chromium.org Reviewed-on: https://chromium-review.googlesource.com/193001 (cherry picked from commit 671f82fd5963e32e72d3886aa242cb3e8519f226) Signed-off-by: Isaac Christensen isaac.christensen@se-eng.com --- src/lib/edid.c | 61 +++++++++++++++------------------------------------------- 1 file changed, 16 insertions(+), 45 deletions(-)
diff --git a/src/lib/edid.c b/src/lib/edid.c index 6bd471a..3b312ba 100644 --- a/src/lib/edid.c +++ b/src/lib/edid.c @@ -47,7 +47,6 @@ static int nonconformant_digital_display = 0; static int nonconformant_extension = 0; static int did_detailed_timing = 0; static int has_name_descriptor = 0; -static int name_descriptor_terminated = 0; static int has_range_descriptor = 0; static int has_preferred_timing = 0; static int has_valid_checksum = 0; @@ -146,18 +145,9 @@ detailed_cvt_descriptor(struct edid *out, unsigned char *x, int first) return valid; }
-static int isalnum(char x) -{ - if (x >= 'a' && x <= 'z') - return 1; - if (x >= 'A' && x <= 'Z') - return 1; - if (x >= '0' && x <= '9') - return 1; - return 0; - -} -/* extract a string from a detailed subblock, checking for termination */ +/* extract a CP437 string from a detailed subblock, checking for termination (if + * less than len of bytes) with LF and padded with SP. + */ static char * extract_string(unsigned char *x, int *valid_termination, int len) { @@ -167,20 +157,16 @@ extract_string(unsigned char *x, int *valid_termination, int len) memset(ret, 0, sizeof(ret));
for (i = 0; i < len; i++) { - if (isalnum(x[i])) { - ret[i] = x[i]; - } else if (!seen_newline) { - if (x[i] == 0x0a) { - seen_newline = 1; - } else { - *valid_termination = 0; - return ret; - } - } else { + if (seen_newline) { if (x[i] != 0x20) { *valid_termination = 0; return ret; } + } else if (x[i] == 0x0a) { + seen_newline = 1; + } else { + /* normal characters */ + ret[i] = x[i]; } }
@@ -191,7 +177,6 @@ extract_string(unsigned char *x, int *valid_termination, int len) static int detailed_block(struct edid *out, unsigned char *x, int in_extension) { - static unsigned char name[53]; int i; #if 1 printk(BIOS_SPEW, "Hex of detail: "); @@ -262,21 +247,10 @@ detailed_block(struct edid *out, unsigned char *x, int in_extension) printk(BIOS_SPEW, "Color point\n"); return 0; case 0xFC: - /* XXX should check for spaces after the \n */ - /* XXX check: terminated with 0x0A, padded with 0x20 */ - has_name_descriptor = 1; - if (strchr((char *)name, '\n')) return 0; - /* avoid strncat - strncat((char *)name, (char *)x + 5, 13); - */ - if (strchr((char *)name, '\n') || strchr((char *)x+5, '\n')) { - name_descriptor_terminated = 1; - /* later. - printk(BIOS_SPEW, "Monitor name: %s\n", - extract_string(name, &has_valid_string_termination, - strlen((char *)name))); - */ - } + printk(BIOS_SPEW, "Monitor name: %s\n", + extract_string(x + 5, + &has_valid_string_termination, + 13)); return 0; case 0xFD: { @@ -1282,7 +1256,6 @@ int decode_edid(unsigned char *edid, int size, struct edid *out) !has_valid_string_termination || !has_valid_descriptor_pad || !has_name_descriptor || - !name_descriptor_terminated || !has_preferred_timing || !has_range_descriptor) conformant = 0; @@ -1293,8 +1266,6 @@ int decode_edid(unsigned char *edid, int size, struct edid *out) nonconformant_digital_display); if (!has_name_descriptor) printk(BIOS_ERR, "\tMissing name descriptor\n"); - else if (!name_descriptor_terminated) - printk(BIOS_ERR, "\tName descriptor not terminated with a newline\n"); if (!has_preferred_timing) printk(BIOS_ERR, "\tMissing preferred timing\n"); if (!has_range_descriptor) @@ -1305,15 +1276,15 @@ int decode_edid(unsigned char *edid, int size, struct edid *out) printk(BIOS_ERR, "\tDetailed block string not properly terminated\n"); } else if (claims_one_point_two) { if (nonconformant_digital_display || - (has_name_descriptor && !name_descriptor_terminated)) + !has_valid_string_termination) conformant = 0; if (!conformant) printk(BIOS_ERR, "EDID block does NOT conform to EDID 1.2!\n"); if (nonconformant_digital_display) printk(BIOS_ERR, "\tDigital display field contains garbage: %x\n", nonconformant_digital_display); - if (has_name_descriptor && !name_descriptor_terminated) - printk(BIOS_ERR, "\tName descriptor not terminated with a newline\n"); + if (!has_valid_string_termination) + printk(BIOS_ERR, "\tDetailed block string not properly terminated\n"); } else if (claims_one_point_oh) { if (seen_non_detailed_descriptor) conformant = 0;