Page MenuHomeFreeBSD

bnxt: Handle errors from copyout() in ioctl handlers
ClosedPublic

Authored by markj on Dec 25 2023, 6:12 PM.
Tags
None
Referenced Files
F103921638: D43178.diff
Sun, Dec 1, 6:22 AM
Unknown Object (File)
Thu, Nov 14, 9:11 PM
Unknown Object (File)
Sun, Nov 3, 1:29 AM
Unknown Object (File)
Oct 5 2024, 7:58 AM
Unknown Object (File)
Oct 2 2024, 6:07 AM
Unknown Object (File)
Sep 27 2024, 3:30 PM
Unknown Object (File)
Sep 23 2024, 2:26 PM
Unknown Object (File)
Sep 18 2024, 7:36 PM
Subscribers
None

Details

Summary

This is in preparation for annotating copyin() and related functions
with __result_use_check.

Diff Detail

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

Event Timeline

markj requested review of this revision.Dec 25 2023, 6:12 PM
sys/dev/bnxt/if_bnxt.c
2571

Why the void cast here and below?

sys/dev/bnxt/if_bnxt.c
2571

In this code path, the ioctl is already going to return an error (rc != 0). Here, we are copying out the return code directly for some reason; if that fails, I don't want to clobber the existing error. That's a general pattern that driver ioctl handlers tend to follow.

(I should have made this more clear in the commit message and will amend it.)

imp added inline comments.
sys/dev/bnxt/if_bnxt.c
2571

ah yes. so it does. A mention in the commit is fine.

This revision is now accepted and ready to land.Dec 26 2023, 2:04 AM
sys/dev/bnxt/if_bnxt.c
2571

I realized that the new behaviour is a bit incompatible with the old one. Before this patch, if the ioctl returned an error, we'd copy the error out and then return 0 (the old code always sets rc = 0). That's a weird convention, but it's not my intention to change it.

Preserve old error handling semantics. If the ioctl operation fails,
we copy out the error number. If that fails, we return the error from
copyout().

This revision now requires review to proceed.Dec 28 2023, 5:01 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jan 4 2024, 1:41 PM
This revision was automatically updated to reflect the committed changes.