Page MenuHomeFreeBSD

camcontrol(8): remove unnecessary CCB zeroing
ClosedPublic

Authored by trasz on Dec 29 2020, 5:19 PM.

Details

Summary

After 3e404b8c53d, cam_getccb(3) clears the returned CCB, making
a number of calls to CCB_CLEAR_ALL_EXCEPT_HDR(3) unnecessary.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

trasz requested review of this revision.Dec 29 2020, 5:19 PM
trasz created this revision.

All but one of these are easily seen to be redundant now.

sbin/camcontrol/camcontrol.c
1828

This one I'm not sure of since I've not traced all callers of ata_do_pass_16

This revision is now accepted and ready to land.Feb 24 2021, 10:37 PM
imp requested changes to this revision.Feb 24 2021, 10:46 PM

I'd commit all but the flagged one.
The flagged one, if someone can confirm my code tracing, changes things. Don't know if the change is a breakage or just a benign change, but I think it requires additional study (and maybe a comment about why it's needed, or the flow needs to change to call it after the flagged ata_do_identify, but even then I've not chased down all branches.

sbin/camcontrol/camcontrol.c
1828

I'm pretty sure this isn't good.

atahpa() gets a ccb
Then calls ata_do_identify (which fills in the ccb)
And later atahpa can call atahpa_password when action is set to ATA_HPA_ACTION_SET_PWD
which calls ata_do_cmd
which will call ata_do_pass_16 if ata_try_pass_16
which used to call CCB_CLEAR_ALL_EXCEPT_HDR to zero everything out, but now does not do so after the identify already stomped on things in this ccb. Don't know if the stomping is OK or not, but this is a change.

This revision now requires changes to proceed.Feb 24 2021, 10:46 PM

Add back chunk that's required, as advised by imp@.

Filter the unrelated things out, please, and we're good.

sbin/camcontrol/camcontrol.c
113

This is a good change, but unrelated.

250–251

ditto

10241

ditto

This revision is now accepted and ready to land.Feb 24 2021, 11:05 PM