camcontrol: possibly wrong sector count in ata_do_identify()

[danfe]

Summary

I've noticed that in the ata_do_identify() function that was added in rS249115 issues ATA_ATA_IDENTIFY (0xec) command which reads a sector (512 bytes) of drive identification data (struct ata_params), but passes (u_int8_t)sizeof(struct ata_params) as sector_count which evaluates to zero.

This looks at least bogus, if not wrong. All the similar code I've seen, including e.g. that of popular Linux hdparm utility, supply 1 (one) as a sector count in this case. Even if zero is correct, it should not be obfuscated as a cast of 512 to u_int8_t which is too large and silently gets truncated.

I've tried to contact @smh several times about the matter, but he never replied, so apparently he's no longer active with the project, thus I'm putting it up for review here.

[smh]

Sorry for lack of responses, I did see your requests but got lost in work stuff.

I've tested with zero for both standard ata and scsi ata pass through with no ill effects and as the ata spec defines the value as N/A I believe a zero is the more correct value.

If no objections I'll commit that change later today.

camcontrol.c

2295 ↗

(On Diff #54563)

I believe 0 is actually the correct value in both case as both lba and count are ignored by the identify command.

[danfe]

camcontrol.c

2295 ↗

(On Diff #54563)

I confirmed with the spec, it indeed specifies N/A, although I didn't find how they suggest N/A to be translated in implementations (code). That said, I'm fine with zero.

[mav]

I agree, zero should be the right answer here. That is what kernel does also.

[imp]

I figured that this was OK, but just checked this am and I noticed this commit.
Just wanted to say that it looks good to me too.
Thanks for driving this home @smh