Hi Elliott,
Thanks for the input!
On 05/04/2021 18:01, Elliott Mitchell wrote:
On Mon, Apr 05, 2021 at 04:56:59PM +0100, Julien Grall wrote:
I am not aware of code trying to modify literal strings in Xen.
However, there is a frequent use of non-const char * to point to
literal strings. Given the size of the codebase, there is a risk
to involuntarily introduce code that will modify literal strings.
Therefore it would be better to enforce using const when pointing
to such strings. Both GCC and Clang provide an option to warn
for such case (see -Wwrite-strings) and therefore could be used
by Xen.
This series doesn't yet make use of -Wwrite-strings because
the tree is not fully converted. Instead, it contains some easy
and likely non-controversial use const in the code.
The major blockers to enable -Wwrite-strings are the following:
- xen/common/efi: union string is used in both const and
non-const situation. It doesn't feel right to specific one member
const and the other non-const.
- libxl: the major block is the flexarray framework as we would use
it with string (now const char*). I thought it would be possible to
make the interface const, but it looks like there are a couple of
places where we need to modify the content (such as in
libxl_json.c).
Ideally, I would like to have -Wwrite-strings unconditionally used
tree-wide. But, some of the area may required some heavy refactoring.
One solution would be to enable it tree-wide but turned it off at a
directroy/file level.
Any opinions?
I think doing such is a Good Idea(tm). Adding consts adds opportunities
for greater optimization. Both by compilers which can avoid extra
copies, and developers who then know they don't need to generate extra
copies to sacrific them to an API. In particular the consts also
function as documentation.
So you're certainly not the only person who thinks additional consts
would be a good thing:
https://lists.xenproject.org/archives/html/xen-devel/2021-01/msg00132.html
Alas merely getting the two const patches into the latest release
apparently wasn't even worthy of response:
https://lists.xenproject.org/archives/html/xen-devel/2021-02/msg01040.html
Sory to hear you had no response. Would you be able to give a nudge to
the maintainers?
I agree this should be done, though I'm aware many developers hate
bothering with constants.
Cheers,
--
Julien Grall