The iflib core never modifies the isc_driver_version string. Allow
drivers to safely assign pointers to constant buffers by marking this
parameter const.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Paths
| Differential D19577 Authored by jacob.e.keller_intel.com on Mar 13 2019, 10:41 PM.
Tags None Referenced Files
Details
Summary The iflib core never modifies the isc_driver_version string. Allow Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Diff Detail
Event TimelineHerald added a reviewer: shurd. · View Herald TranscriptMar 13 2019, 10:41 PM2019-03-13 22:41:54 (UTC+0) Harbormaster completed remote builds in B23073: Diff 55040.Mar 13 2019, 10:41 PM2019-03-13 22:41:54 (UTC+0) jacob.e.keller_intel.com added reviewers: erj, gallatin.Mar 13 2019, 10:42 PM2019-03-13 22:42:33 (UTC+0) Comment ActionsDrivers can sort of get around this by using an explicit cast, but that triggers a -Wcast-qual warning. This revision is now accepted and ready to land.Mar 13 2019, 11:40 PM2019-03-13 23:40:11 (UTC+0) Comment Actions Uhh, oh I see why it's marked as non-const.... SYSCTL_ADD_STRING with a read-only flag doesn't know to take a const value.... jacob.e.keller_intel.com planned changes to this revision.Mar 13 2019, 11:44 PM2019-03-13 23:44:42 (UTC+0) Comment ActionsThis needs some re-work before it's acceptable. jacob.e.keller_intel.com updated this revision to Diff 55042.Mar 14 2019, 12:02 AM2019-03-14 00:02:56 (UTC+0) Comment ActionsAlso cast isc_driver_version in SYSCTL_ADD_STRING Harbormaster completed remote builds in B23075: Diff 55042.Mar 14 2019, 12:02 AM2019-03-14 00:02:58 (UTC+0) jacob.e.keller_intel.com planned changes to this revision.Mar 14 2019, 12:03 AM2019-03-14 00:03:42 (UTC+0) Comment ActionsThis still doesn't quite work 100%, because it causes a -Wcast-qual warning.... I'm not really sure how to fix this without using clang diagnostics. Comment Actions Looks like the best 'fix' would be to use _Pragma to drop the warnings in SYSCTL_ADD_STRING. That will take a bit more work to get correct. Comment Actions
Probably through the addition of some sort of "__force_cast()" macro which expands to a pragma to ignore the warnings, the cast, and then the pragma to pop the ignoring... Herald added a reviewer: iflib. · View Herald TranscriptMar 14 2019, 9:02 PM2019-03-14 21:02:31 (UTC+0) jacob.e.keller_intel.com updated this revision to Diff 55079.Mar 14 2019, 10:40 PM2019-03-14 22:40:42 (UTC+0) Comment ActionsIntroduce a __drop_const macro to handle the warning Harbormaster completed remote builds in B23095: Diff 55079.Mar 14 2019, 10:40 PM2019-03-14 22:40:45 (UTC+0) Comment Actions This version actually compilers without the warnings, but it *is* rather ugly to have to do it this way
Comment Actions I think I'd rather have a SYSCTL_ADD_CONST_STRING or the like, but presumably you'd just get the warning again. The other hack is to cast through uintptr_t and it's what we normally do via the __DECONST() macro. I would just do that unconditionally perhaps: char *__arg = __DECONST(arg, char *); Comment Actions
I didn't know about __DECONST. I'll take a look at it, thanks! Yea, we could add a SYSCTL_ADD_CONST_STRING as well to be more explicit. jacob.e.keller_intel.com updated this revision to Diff 55081.Mar 14 2019, 11:40 PM2019-03-14 23:40:14 (UTC+0) Comment ActionsUse __DECONST instead of a poor reimplementation Introduce SYSCTL_CONST_STRING and SYSCTL_ADD_CONST_STRING Harbormaster completed remote builds in B23096: Diff 55081.Mar 14 2019, 11:40 PM2019-03-14 23:40:17 (UTC+0) Comment Actions I like the addition of SYSCTL_ADD_CONST_STRING since it also ensures that we enforce that we don't have CTLFLAG_WR.
Comment Actions Just need to add the new macros to sysctl.9 and document them. I think I like having the explicit const string macros. jacob.e.keller_intel.com updated this revision to Diff 55083.Mar 15 2019, 12:38 AM2019-03-15 00:38:18 (UTC+0) Comment ActionsDocument the new CONST_STRING sysctl macros Harbormaster completed remote builds in B23098: Diff 55083.Mar 15 2019, 12:38 AM2019-03-15 00:38:18 (UTC+0) This revision is now accepted and ready to land.Mar 19 2019, 11:24 PM2019-03-19 23:24:01 (UTC+0) Closed by commit rS345312: iflib: mark isc_driver_version as constant (authored by erj). · Explain WhyMar 19 2019, 11:44 PM2019-03-19 23:44:33 (UTC+0) This revision was automatically updated to reflect the committed changes. Herald added a reviewer: manpages. · View Herald TranscriptMar 19 2019, 11:44 PM2019-03-19 23:44:33 (UTC+0)
Revision Contents
Diff 55083 share/man/man9/sysctl.9
sys/net/iflib.h
sys/net/iflib.c
sys/sys/sysctl.h
|
This is a xdma_desc_dmamap_cb in , the name is too short and nondescript. I do not think descriptor allocation belongs in xdma at all, but this is wrong nonetheless.