Re: CVS commit: src/sys/arch/atari/stand/installboot

[christos%zoulas.com@localhost (Christos Zoulas)]

On Nov 24,  2:28am, tsutsui%ceres.dti.ne.jp@localhost (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;
 }