Hi Mark, I haven't looked at the code in detail, I'm afraid. There's a general suggestion, which should be useful.
As the commit summary indicates, this should be two separate patches. On 9 December 2016 at 17:39, Mark Ferry <[email protected]> wrote: > --- > edid-decode.c | 40 +++++++++++++++++++++++++--------------- > 1 file changed, 25 insertions(+), 15 deletions(-) > > diff --git a/edid-decode.c b/edid-decode.c > index c18697f..6df2b6e 100644 > --- a/edid-decode.c > +++ b/edid-decode.c > @@ -47,6 +47,7 @@ static int has_range_descriptor = 0; > static int has_preferred_timing = 0; > static int has_valid_checksum = 1; > static int has_valid_cvt = 1; > +static int has_valid_displayid_checksum = 1; Patch 2/2 alongside ... > static int has_valid_dummy_block = 1; > static int has_valid_week = 0; > static int has_valid_year = 0; > @@ -560,23 +561,27 @@ detailed_block(unsigned char *x, int in_extension) > return 1; > } > > -static void > -do_checksum(unsigned char *x) > +static unsigned char > +do_checksum(unsigned char *x, size_t len) > { > - printf("Checksum: 0x%hx", x[0x7f]); > - { > - unsigned char sum = 0; > - int i; > - for (i = 0; i < 128; i++) > - sum += x[i]; > - if (sum) { > - printf(" (should be 0x%hx)", (unsigned char)(x[0x7f] - sum)); > - has_valid_checksum = 0; > - } else printf(" (valid)"); > - } > + unsigned char sum = 0; > + int i; > + > + printf("Checksum: 0x%hx", x[len -1]); > + > + for (i = 0; i < len; i++) > + sum += x[i]; > + > + if (sum) { > + printf(" (should be 0x%hx)", (unsigned char)(x[len-1] - sum)); > + } else printf(" (valid)"); > + > printf("\n"); > + > + return sum; > } > > + Spurious white space change ? In your 1/2 you might want to mention what, why and/or how do_checksum is refactored. > /* CEA extension */ > > static const char * > @@ -1281,7 +1286,7 @@ parse_cea(unsigned char *x) > detailed_block(detailed, 1); > } while (0); > > - do_checksum(x); > + (void) do_checksum(x, 128); > > return ret; > } > @@ -1371,6 +1376,9 @@ parse_displayid(unsigned char *x) > int ext_count = x[4]; > int i; > printf("Length %d, version %d, extension count %d\n", length, version, > ext_count); > + > + has_valid_displayid_checksum = (do_checksum(x+1, length + 5) == 0x0); > + ... this ... > @@ -2127,6 +2135,8 @@ int main(int argc, char **argv) > printf("\tInvalid detailed timing descriptor ordering\n"); > if (!has_valid_range_descriptor) > printf("\tRange descriptor contains garbage\n"); > + if (!has_valid_displayid_checksum) > + printf("\tBlock has broken DisplayID checksum\n"); ... and this one. Regards, Emil _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
