[PATCH] hdparm size reduction

[Rob Landley]

On Tuesday 02 May 2006 7:50 am, Tito wrote:
> the last version of hdparm just ripped out the dead code
>
> and reinserted the checksumming:
> > > 	/* calculate checksum over all bytes */
> > > 	for(ii = GEN_CONFIG; ii<=INTEGRITY; ii++) {
> > > 		chksum += val[ii] + (val[ii] >> 8);
> > > 	}

This is what the mainstream hdparm is doing, then?

Not much of a chksum, is it?

> Maybe we can try, but later on in the code we have this checks to match:
>
> 		else if (((std == 4) || (!std && (like_std < 5))) &&
> 			((((val[INTEGRITY]	& SIG) == SIG_VAL) && !chksum) ||
> 			((	val[HWRST_RSLT] & VALID) == VALID_VAL) ||
> 			(((	val[CMDS_SUPP_1] & VALID) == VALID_VAL) &&
> 			((	val[CMDS_SUPP_1] & CMDS_W83) > 0x001f)) ) )
> and
>
> 			printf("Checksum: ");
> 			if_printf(chksum,"in");
> 			printf("correct\n");
>
> so it seems we are testing for zero or non zero.

What exactly is the use case for this?  I haven't got the half-hour to read 
through this and try to untangle the logic right now.  (Bit of a mess, isn't 
it?)  What command line would trigger this output, or make a difference in 
the test?

Thirty seconds with the man page proved unrevealing.  Doesn't seem mention a 
checksum at all...

> Could this be done with the new crc32 stuff and if yes how,
> I'm willing to test it if you give me a hint how ;-)

I think the problem here is definitely a question of divining intent, rather 
than anything to do with the implementation.  (And you wonder why I prefer 
implementing sane applets from the specs or man pages, rather than porting in 
external code.  Oh well...)

Rob
-- 
Never bet against the cheap plastic solution.