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

[Mark Ferry]

Thanks for the feedback Walter. Comments below.

On Mon, 12 Dec 2016 20:02:08 +0100, walter harms wrote:
> 
> 
> Am 10.12.2016 20:44, schrieb Mark Ferry:
> > +    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");
> 
> 	any reason not to move the \n to the printf in front ?
> 	just thinking of it ..
> 	perhaps this can be reduced to invalid/valid ? i do not know anyone
> 	seeing more in that information.
> 	then you can simply return 0/-1.	
> 

Agree about the newline and a boolean return value.

I definitely want to keep the "should be 0xxx" output.
Hacking EDIDs and having to recalculate the checksum myself is what
motivated this patch. Not sure if that's what you meant.

> >  
> > -    do_checksum(edid);
> > +    (void) do_checksum(edid, 128);
> >  
> in General i am a fan of checking return values,
> why not here ?

Thanks you're quite right, and I've failed to set has_valid_checksum.

> and why the magic 128 ?
> Perhaps we can avoid the global ?
> 

There are plenty of magic numbers throughout which I'm not about to
clean up in this patch. But I'll take the opportunity to create
EDID_PAGE_SIZE for 128.

Updated patch to follow.

- Mark

-- 
Cognomen Ltd
http://cognomen.co.uk
+44 7855 790184
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20161213/6e7b9a67/attachment.sig>