On the success path the function return (0); without assigning *ret_val.
The existing comment says the function “places in ret_val what should be returned.”
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
ret_val is only considered by the caller if ctf_drop_checks() returns 1. So there is no problem in leaving it unassigned when returning 0. So the proposed fix would not change the behavior. So I would tend to not add the proposed change, but let us get glebius into the loop. Maybe improving the comment is the best way forward.
Comment Actions
Yes, patch as is doesn't improve anything. The comment above the function clearly marks the intent: the function checks if packet should be dropped and if so provides error code. It doesn't provide error code in case packet shouldn't be dropped.
P.S. For the sake of human comprehension I would make the function bool and rename the ret_val to error.
Comment Actions
It might make sense to rename ret_val to locked, which is boolean. This would apply to a couple of functions, The same for changing the type to bool. I can take a look...