[edid-decode] [PATCH 2/2] Calculate DisplayID checksums. Refactor do_checksum.

[Emil Velikov]

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 <mark at cognomen.co.uk> 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