[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


Ian, Wei, Anthony, can I get some feedbacks on the tools side?


On 05/04/2021 16:56, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>

Hi all,

By default, both Clang and GCC will happily compile C code where
non-const char * point to literal strings. This means the following
code will be accepted:

     char *str = "test";

     str[0] = 'a';

Literal strings will reside in rodata, so they are not modifiable.
This will result to an permission fault at runtime if the permissions
are enforced in the page-tables (this is the case in Xen).

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

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?


Julien Grall (14):
   xen: Constify the second parameter of rangeset_new()
   xen/sched: Constify name and opt_name in struct scheduler
   xen/x86: shadow: The return type of sh_audit_flags() should be const
   xen/char: console: Use const whenever we point to literal strings
   tools/libs: guest: Use const whenever we point to literal strings
   tools/libs: stat: Use const whenever we point to literal strings
   tools/xl: Use const whenever we point to literal strings
   tools/firmware: hvmloader: Use const in __bug() and __assert_failed()
   tools/console: Use const whenever we point to literal strings
   tools/kdd: Use const whenever we point to literal strings
   tools/misc: Use const whenever we point to literal strings
   tools/top: The string parameter in set_prompt() and set_delay() should
     be const
   tools/xenmon: xenbaked: Mark const the field text in stat_map_t
   tools/xentrace: Use const whenever we point to literal strings

  tools/console/client/main.c         |  4 +-
  tools/console/daemon/io.c           | 10 ++--
  tools/debugger/kdd/kdd.c            | 10 ++--
  tools/firmware/hvmloader/util.c     |  4 +-
  tools/firmware/hvmloader/util.h     |  4 +-
  tools/include/xenguest.h            | 10 ++--
  tools/libs/guest/xg_dom_core.c      |  8 ++--
  tools/libs/guest/xg_dom_elfloader.c |  4 +-
  tools/libs/guest/xg_dom_hvmloader.c |  2 +-
  tools/libs/guest/xg_dom_x86.c       |  9 ++--
  tools/libs/guest/xg_private.h       |  2 +-
  tools/libs/stat/xenstat_linux.c     |  4 +-
  tools/libs/stat/xenstat_qmp.c       | 12 ++---
  tools/misc/xen-detect.c             |  2 +-
  tools/misc/xenhypfs.c               |  6 +--
  tools/xenmon/xenbaked.c             |  2 +-
  tools/xentop/xentop.c               | 12 ++---
  tools/xentrace/xenalyze.c           | 71 +++++++++++++++--------------
  tools/xentrace/xenctx.c             |  4 +-
  tools/xl/xl.h                       |  8 ++--
  tools/xl/xl_console.c               |  2 +-
  tools/xl/xl_utils.c                 |  4 +-
  tools/xl/xl_utils.h                 |  4 +-
  xen/arch/x86/mm/shadow/multi.c      | 12 ++---
  xen/common/rangeset.c               |  2 +-
  xen/common/sched/private.h          |  4 +-
  xen/drivers/char/console.c          |  4 +-
  xen/include/xen/rangeset.h          |  2 +-
  28 files changed, 113 insertions(+), 109 deletions(-)

Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.