Re: [PATCH 00/14] Use const whether we point to literal strings (take 1)

[Julien Grall <julien@xxxxxxx>]

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