Some of these probably should not be fixed because we simply inherit them
from upstream vendor code. Further still, some should perhaps not be fixed
due to altering API. I would like an opinion on which ones should be fixed
and which should perhaps be left alone.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Hrm. Phabricator seems to think there's no context for your diff here. What's that all about?
sys/dev/ixgbe/ixgbe_phy.h | ||
---|---|---|
85 ↗ | (On Diff #44486) | I think its fine to just rename this without the follow on redefinition for compatibility. I don't think there's a consumer of this outside of the code you've already changed. |
I had to do "arc diff --create --less-context" because otherwise (without the "--less-context" option) arc refuses to post the diff because sys/dev/qlnx/qlnxe/reg_addr.h is 15M in size. Arcanist gives an error stating that it thinks that the diff is wrong because it is too large and that to post the diff you need to pass "--less-context".
sys/dev/ixgbe/ixgbe_phy.h | ||
---|---|---|
85 ↗ | (On Diff #44486) | Would that go for all of the instances where I renamed a macro? Also, since this is in the ixgbe(4) device driver, don't we have to worry about potentially having to make this change everytime we sync with vendor code? Maybe in these cases we should strive to instead get the fix upstreamed so we can inherit the fix on the next sync with vendor? |
I would like an opinion on which ones should be fixed and which should perhaps be left alone.
I'd like every one of them to ultimately be fixed. Fix the low-hanging ones first. Submit patches to the vendors of the code we're using, make sure that they apply it.
API ones are the hardest. Those that are private or internal to FreeBSD should be fixed I think. For public ones that could be used by the rest of the world we should probably follow our usual deprecation routine.
Low-hanging candidates, all non-API changes:
sys/dev/cx/cxddk.c (a comment) sys/dev/drm2/radeon/atombios.h (comments) sys/i386/include/cserial.h (a comment) sys/net80211/ieee80211_hwmp.c (a string) sys/netinet/sctp_indata.c (comments)
Vendor fix required:
contrib/ntp/util/tg.c
from vendor/ntp/dist/util/tg.c (see roberto, cy)
contrib/ntp/util/tg2.c
from vendor/ntp/dist/util/tg2.c (see roberto, cy)
contrib/ofed/opensm/man/opensm.8
from git://git.openfabrics.org/~halr/opensm.git (private; see hselasky)
sys/contrib/alpine-hal/al_hal_pcie.c
from vendor-sys/alpine-hal/dist/al_hal_pcie.c (see zbb, wma)
sys/contrib/octeon-sdk/cvmx-agl-defs.h (see jmallett)
sys/contrib/octeon-sdk/cvmx-asxx-defs.h (see jmallett)
sys/contrib/octeon-sdk/cvmx-gmxx-defs.h (see jmallett)
sys/dev/ixgbe/ixgbe_phy.c (see erj)
sys/dev/ixgbe/ixgbe_phy.h (see erj)
sys/dev/qlnx/qlnxe/common_hsi.h (see davidcs)
sys/dev/qlnx/qlnxe/qlnx_os.c (see davidcs)
sys/dev/qlnx/qlnxe/reg_addr.h (see davidcs)
atombios.h is 3rd party code, although I am unsure if there's a viable upstream to submit it to. Please go ahead with the others for now though.