Page MenuHomeFreeBSD

We pass a pointer to the flags to dabitsysctl, not an integer. Adjust the handler to accept a poitner to a da_flags.
ClosedPublic

Authored by imp on Feb 21 2020, 9:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 15, 1:13 PM
Unknown Object (File)
Tue, Oct 29, 9:23 AM
Unknown Object (File)
Oct 18 2024, 3:36 AM
Unknown Object (File)
Oct 15 2024, 7:41 PM
Unknown Object (File)
Oct 4 2024, 10:19 PM
Unknown Object (File)
Oct 1 2024, 9:30 PM
Unknown Object (File)
Oct 1 2024, 5:28 AM
Unknown Object (File)
Sep 24 2024, 2:21 AM
Subscribers

Details

Summary

We pass a pointer to the flags to dabitsysctl, not an integer. Adjust the
handler to accept a poitner to a u_int. To make the type of the softc flags
stable and defined, make it a u_int. Cast the enum types to u_int for arg2 so
when passing to dabitsysctl it's a u_int.

Noticed by: emax@

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 29525
Build 27392: arc lint + arc unit

Event Timeline

The change looks more correct than the existing logic, but I don't think an enum pointer type is right either.

sys/cam/scsi/scsi_da.c
2622–2623

da_flags is an enum type and probably wrong for an actual flags field that could have multiple bits set (same issue in the softc::flags field).

If anything, test should be of type da_flags, and flags not.

imp added inline comments.
sys/cam/scsi/scsi_da.c
2622–2623

da_flags is an enum type and probably wrong for an actual flags field that could have multiple bits set (same issue in the softc::flags field).

enum in C++ acts that way. enum on C is just an int-ish type. this is a source of much grief in my past lives...

If anything, test should be of type da_flags, and flags not.

I think test should be as well...

However, I'm thinking that making it all u_int might actually be better (all here is softc flags, and the two items here since that would let us hoist this function into sysctl.

LGTM.

sys/cam/scsi/scsi_da.c
2622–2623

I don't know if you're right about the language (you probably are) but compilers (largely written in C++) and static analysis tools are starting to assume things like "only defined enum values are valid values for an enum variable" so IMO it's probably easier to just follow the same model.

This revision is now accepted and ready to land.Feb 21 2020, 10:31 PM