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
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
