From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH 6/8] xen/x86: Avoid overriding initialisers
 in arrays
Date: Wed, 10 Feb 2016 06:22:55 -0700
Message-ID: <56BB47BF02000078000D08FC@prv-mh.provo.novell.com>
References: <1455048108-5045-1-git-send-email-andrew.cooper3@citrix.com>
	<1455048108-5045-7-git-send-email-andrew.cooper3@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1455048108-5045-7-git-send-email-andrew.cooper3@citrix.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, Kevin Tian <kevin.tian@intel.com>, JunNakajima <jun.nakajima@intel.com>, Xen-devel <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

>>> On 09.02.16 at 21:01, <andrew.cooper3@citrix.com> wrote:
> Clang objects to having multiple initialisers when creating an array.
> 
> As this warning is useful for spotting obscure bugs, disabling it is
> unhelpful.  Instead, fix our two deliberate usecases.

Ugly again, but - well ...

> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1201,6 +1201,20 @@ void ept_p2m_uninit(struct p2m_domain *p2m)
>      free_cpumask_var(ept->invalidate);
>  }
>  
> +static const char *memory_type_to_str(unsigned int x)
> +{
> +    static const char memory_types[8][2] = {
> +        [MTRR_TYPE_UNCACHABLE]     = "UC",
> +        [MTRR_TYPE_WRCOMB]         = "WC",
> +        [MTRR_TYPE_WRTHROUGH]      = "WT",
> +        [MTRR_TYPE_WRPROT]         = "WP",
> +        [MTRR_TYPE_WRBACK]         = "WB",
> +        [MTRR_NUM_TYPES]           = "??"
> +    };
> +
> +    return x < ARRAY_SIZE(memory_types) ? (memory_types[x] ?: "?") : "?";

I think this should really ASSERT() the first condition.

> @@ -1212,15 +1226,6 @@ static void ept_dump_p2m_table(unsigned char key)
>      unsigned long record_counter = 0;
>      struct p2m_domain *p2m;
>      struct ept_data *ept;
> -    static const char memory_types[8][2] = {
> -        [0 ... 7] = "?",
> -        [MTRR_TYPE_UNCACHABLE]     = "UC",
> -        [MTRR_TYPE_WRCOMB]         = "WC",
> -        [MTRR_TYPE_WRTHROUGH]      = "WT",
> -        [MTRR_TYPE_WRPROT]         = "WP",
> -        [MTRR_TYPE_WRBACK]         = "WB",
> -        [MTRR_NUM_TYPES]           = "??"
> -    };
>  
>      for_each_domain(d)
>      {
> @@ -1260,8 +1265,8 @@ static void ept_dump_p2m_table(unsigned char key)
>                             ept_entry->r ? 'r' : ' ',
>                             ept_entry->w ? 'w' : ' ',
>                             ept_entry->x ? 'x' : ' ',
> -                           memory_types[ept_entry->emt][0],
> -                           memory_types[ept_entry->emt][1]
> +                           memory_type_to_str(ept_entry->emt)[0],
> +                           memory_type_to_str(ept_entry->emt)[1]
>                             ?: ept_entry->emt + '0',
>                             c ?: ept_entry->ipat ? '!' : ' ');

There's actually a bug here, which I think is worth fixing at once:
The default initializer was a string of length 1, resulting in a
premature NUL character to get placed into the fully expanded
string, causing - afaict - truncation of the intended message. I
therefore think the default string should be e.g. "? ".

Jan