iflib: mark isc_driver_version as constant

[jacob.e.keller_intel.com]

Summary

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 <[e-mail]>

[jacob.e.keller_intel.com]

Drivers can sort of get around this by using an explicit cast, but that triggers a -Wcast-qual warning.

[jacob.e.keller_intel.com]

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]

This needs some re-work before it's acceptable.

[jacob.e.keller_intel.com]

Also cast isc_driver_version in SYSCTL_ADD_STRING

[jacob.e.keller_intel.com]

This 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.

[jacob.e.keller_intel.com]

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.

[jacob.e.keller_intel.com]

In D19577#419066, @jacob.e.keller_intel.com wrote:

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.

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...

[jacob.e.keller_intel.com]

Introduce a __drop_const macro to handle the warning

[jacob.e.keller_intel.com]

This version actually compilers without the warnings, but it *is* rather ugly to have to do it this way

[jacob.e.keller_intel.com]

sys/sys/sysctl.h

349–352 ↗

(On Diff #55079)

This could be reduced to drop_const(arg = (arg)), i'm not sure if that's better or not.

I tried using __builtin_constant_p(len) && len==0 as a way to decide which expression to use, but clang wasn't smart enough to constant-fold that check, so it didn't work.

[jhb]

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 *);

[jacob.e.keller_intel.com]

In D19577#419330, @jhb wrote:

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 *);

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]

Use __DECONST instead of a poor reimplementation

Introduce SYSCTL_CONST_STRING and SYSCTL_ADD_CONST_STRING

[jacob.e.keller_intel.com]

I like the addition of SYSCTL_ADD_CONST_STRING since it also ensures that we enforce that we don't have CTLFLAG_WR.

sys/net/iflib.c

6233–6235 ↗

(On Diff #55081)

The minimal change without the SYSCTL_ADD_CONST_STRING would be to just use __DECONST() here.

[jhb]

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]

Document the new CONST_STRING sysctl macros