if_media.c SIOCGMEDIAX handler: improve loop
ClosedPublic
Actions

Authored by kib on Nov 3 2020, 12:20 PM.

Details

Reviewers

Commits

rS367296: if_media.c SIOCGMEDIAX handler: improve loop

Summary

Stop advancing counter past the current iteration number at the start of iteration. This removes the need of subtracting one when calculating index for copyout, and arguably fixes off-by-one reporting of copied out elements when copyout failed.

Sponsored by: Mellanox Technologies / NVidia Networking

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Nov 3 2020, 12:20 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptNov 3 2020, 12:20 PM

kib requested review of this revision.Nov 3 2020, 12:20 PM

Harbormaster completed remote builds in B34596: Diff 79124.Nov 3 2020, 12:20 PM

• hselasky added inline comments.Nov 3 2020, 12:44 PM

sys/net/if_media.c
312 ↗	(On Diff #79124)	I think this if should just read: if (ep == NULL) --HPS

• hselasky added inline comments.Nov 3 2020, 12:46 PM

sys/net/if_media.c
312 ↗	(On Diff #79124)	if (ep == NULL && i > ifm->ifm_count)

• hselasky accepted this revision.Nov 3 2020, 12:47 PM

This revision is now accepted and ready to land.Nov 3 2020, 12:47 PM

kib added inline comments.Nov 3 2020, 1:10 PM

sys/net/if_media.c
312 ↗	(On Diff #79124)	Do you suggest that this would make any functional change ? I believe it is not, and for me using counters for both sides of 'and' is somewhat cleaner.

• hselasky added inline comments.Nov 3 2020, 1:18 PM

sys/net/if_media.c
312 ↗	(On Diff #79124)	No, it doesn't make any functional change. It just took some time to understand the code. Maybe that if (error != 0) break, should be a goto, because the if (error == 0) is then a NOP. Anyways, I've also accepted the current version.