xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: xen-devel@lists.xenproject.org, Ian Jackson <iwj@xenproject.org>,
	Wei Liu <wl@xen.org>, Anthony PERARD <anthony.perard@citrix.com>
Cc: "Julien Grall" <jgrall@amazon.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Dario Faggioli" <dfaggioli@suse.com>, "Tim Deegan" <tim@xen.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: PING Re: [PATCH 00/14] Use const whether we point to literal strings (take 1)
Date: Mon, 10 May 2021 18:49:01 +0100	[thread overview]
Message-ID: <05eaa910-7630-d1e4-4b70-3008d672fe5c@xen.org> (raw)
In-Reply-To: <20210405155713.29754-1-julien@xen.org>

Hi,

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

Cheers,

On 05/04/2021 16:56, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> 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
>      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?
> 
> Cheers,
> 
> 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


  parent reply	other threads:[~2021-05-10 17:49 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 15:56 [PATCH 00/14] Use const whether we point to literal strings (take 1) Julien Grall
2021-04-05 15:57 ` [PATCH 01/14] xen: Constify the second parameter of rangeset_new() Julien Grall
2021-04-06  7:57   ` Jan Beulich
2021-04-06 18:03     ` Julien Grall
2021-04-05 15:57 ` [PATCH 02/14] xen/sched: Constify name and opt_name in struct scheduler Julien Grall
2021-04-06  8:07   ` Jan Beulich
2021-04-06 18:24     ` Julien Grall
2021-04-07  8:22       ` Jan Beulich
2021-04-07  9:06         ` Julien Grall
2021-04-06 14:19   ` George Dunlap
2021-04-05 15:57 ` [PATCH 03/14] xen/x86: shadow: The return type of sh_audit_flags() should be const Julien Grall
2021-04-06  7:24   ` Roger Pau Monné
2021-04-06 18:26     ` Julien Grall
2021-04-06 14:00   ` Tim Deegan
2021-04-05 15:57 ` [PATCH 04/14] xen/char: console: Use const whenever we point to literal strings Julien Grall
2021-04-06  8:10   ` Jan Beulich
2021-04-06 18:27     ` Julien Grall
2021-04-05 15:57 ` [PATCH 05/14] tools/libs: guest: " Julien Grall
2021-05-11 14:58   ` Anthony PERARD
2021-05-18 13:33     ` Julien Grall
2021-04-05 15:57 ` [PATCH 06/14] tools/libs: stat: " Julien Grall
2021-05-11 15:03   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 07/14] tools/xl: " Julien Grall
2021-04-27 16:04   ` Anthony PERARD
2021-04-27 16:28     ` Julien Grall
2021-04-27 17:03       ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 08/14] tools/firmware: hvmloader: Use const in __bug() and __assert_failed() Julien Grall
2021-04-06  7:29   ` Roger Pau Monné
2021-04-06 19:02     ` Julien Grall
2021-04-05 15:57 ` [PATCH 09/14] tools/console: Use const whenever we point to literal strings Julien Grall
2021-05-11 15:18   ` Anthony PERARD
2021-05-18 13:48     ` Julien Grall
2021-04-05 15:57 ` [PATCH 10/14] tools/kdd: " Julien Grall
2021-04-06 14:03   ` Tim Deegan
2021-04-05 15:57 ` [PATCH 11/14] tools/misc: " Julien Grall
2021-05-11 15:37   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 12/14] tools/top: The string parameter in set_prompt() and set_delay() should be const Julien Grall
2021-05-11 15:46   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 13/14] tools/xenmon: xenbaked: Mark const the field text in stat_map_t Julien Grall
2021-05-11 16:08   ` Anthony PERARD
2021-04-05 15:57 ` [PATCH 14/14] tools/xentrace: Use const whenever we point to literal strings Julien Grall
2021-04-06 14:15   ` George Dunlap
2021-04-05 17:01 ` [PATCH 00/14] Use const whether we point to literal strings (take 1) Elliott Mitchell
2021-04-06 17:55   ` Julien Grall
2021-04-06  7:50 ` Jan Beulich
2021-04-06 19:08 ` Julien Grall
2021-05-10 17:49 ` Julien Grall [this message]
2021-05-17 18:41   ` PING " Wei Liu
2021-05-18 14:02     ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05eaa910-7630-d1e4-4b70-3008d672fe5c@xen.org \
    --to=julien@xen.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=dfaggioli@suse.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=jgrall@amazon.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).