Page MenuHomeFreeBSD

iicbus: add compat32 support for I2C ioctls
ClosedPublic

Authored by stevek on Nov 29 2023, 7:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 7:14 PM
Unknown Object (File)
Sat, Apr 27, 7:14 PM
Unknown Object (File)
Sat, Apr 27, 7:14 PM
Unknown Object (File)
Sat, Apr 27, 7:14 PM
Unknown Object (File)
Sat, Apr 27, 7:14 PM
Unknown Object (File)
Sat, Apr 27, 5:09 PM
Unknown Object (File)
Tue, Apr 23, 5:22 PM
Unknown Object (File)
Sun, Apr 21, 11:36 PM
Subscribers

Details

Summary

Some of the I2C ioctl request structures contain pointers and need to
handle requests from 32-bit applications on 64-bit kernels.

Obtained from: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb added inline comments.
sys/dev/iicbus/iic.c
337–338

I would maybe use iic_ prefix for new functions.

361

Hmm, is the error -Wunused? I would maybe just use __unused instead in the function declaration.

This revision is now accepted and ready to land.Nov 29 2023, 8:57 PM
sys/dev/iicbus/iic.c
521

Maybe similar to what you did in the SMB patch, use a single early switch to do the swabbing so that the code is shared between I2CREAD and I2CWRITE? I prefer that in general (doing all the swabbing before/after the "main" switch on the ioctl cmd) but was just going to let it go until I saw you did this already in the SMB case. :)

sys/dev/iicbus/iic.c
361

Yes, that's for -Wunused. I wasn't sure what is recommended when an argument is used in some cases, but not in others, dependent on defines. I'm fine with using __unused, if that's okay, even though it is used in the case of COMPAT_FREEBSD32 being defined.

Addressed comments from jhb

This revision now requires review to proceed.Nov 30 2023, 5:19 AM
sys/dev/iicbus/iic.c
337–338

Note this was not a new function, just adding an argument. However, iiccopyinmsgs32 is new. I was following the naming convention in the file, but if you want iiccopyinmsgs32 to have iic_copyinmsgs32, I can do that.

Changed iiccopyinmsgs32 to iic_copyinmsgs32, per suggested comments

jhb added inline comments.
sys/dev/iicbus/iic.c
361

I think it's less ugly. If there were several of these variables you could define a new compat32_unused wrapper that is conditional on an #ifdef similar to what we've done for diagused for INVARIANTS, but for one variable the wrapper isn't worth it.

This revision is now accepted and ready to land.Nov 30 2023, 11:51 PM
This revision was automatically updated to reflect the committed changes.