On Nov 24, 2:28am, tsut...@ceres.dti.ne.jp (Izumi Tsutsui) wrote: -- Subject: Re: CVS commit: src/sys/arch/atari/stand/installboot
| christos@ wrote: | | > | http://mail-index.netbsd.org/source-changes-d/2014/11/15/msg007338.html | > | > I could not have tested; I asked you to test. | | I didn't tested it either. | I noticed your obvious wrong pointer calculation by code inspection, | and it means memcpy() would confuse future maintainers "which address | should be used for the cksum." | | > | If you still don't like it, please propose a patch which satisfies | > | both requests as martin said, not repeating your "memcpy is ok and | > | -fstrict-aliasing is more important" claim. | > | > I did not propose that one but the be16enc() solution someone else | > presented seems more natural (and will work on LE machines). | | You still don't understand my point: | "to use u_int16_t assignment consistently." | be16enc() is a bit better them memcpy(), but not ok for me. | | memcpy() and be16enc() are functions to access stream buffers. | The cksum should be an element of u_int16_t array, not part of stream | in this case. How about this then, christos Index: installboot.c =================================================================== RCS file: /cvsroot/src/sys/arch/atari/stand/installboot/installboot.c,v retrieving revision 1.28 diff -u -u -r1.28 installboot.c --- installboot.c 31 Mar 2014 06:32:31 -0000 1.28 +++ installboot.c 23 Nov 2014 18:16:05 -0000 @@ -55,7 +55,7 @@ static void usage(void); static void oscheck(void); -static u_int abcksum(void *); +static void abcksum(void *); static void setNVpref(void); static void setIDEpar(u_int8_t *, size_t); static void mkahdiboot(struct ahdi_root *, char *, @@ -455,8 +455,7 @@ gotit: /* copy code from prototype and set new checksum */ memcpy(newroot->ar_fill, tmproot.ar_fill, sizeof(tmproot.ar_fill)); - newroot->ar_checksum = 0; - newroot->ar_checksum = 0x1234 - abcksum(newroot); + abcksum(newroot); if (verbose) printf("AHDI boot loader: %s\n", xxb00t); @@ -498,8 +497,7 @@ setIDEpar(bb->bb_xxboot, sizeof(bb->bb_xxboot)); /* set AHDI checksum */ - *((u_int16_t *)bb->bb_xxboot + 255) = 0; - *((u_int16_t *)bb->bb_xxboot + 255) = 0x1234 - abcksum(bb->bb_xxboot); + abcksum(bb->bb_xxboot); if (verbose) { printf("Primary boot loader: %s\n", xxb); @@ -571,14 +569,14 @@ } } -static u_int -abcksum (void *bs) +static void +abcksum(void *bs) { u_int16_t sum = 0, *st = (u_int16_t *)bs, - *end = (u_int16_t *)bs + 256; + *end = (u_int16_t *)bs + 255; while (st < end) sum += *st++; - return(sum); + *st++ = 0x1234 - sum; }