From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] xen/x86: Fix errors arising from c/s dab76ff Date: Mon, 15 Feb 2016 15:15:31 +0000 Message-ID: <56C1EB93.4020003@citrix.com> References: <1455289189-32425-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------010306040504080802050805" Return-path: In-Reply-To: <1455289189-32425-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Xen-devel Cc: George Dunlap , Jan Beulich List-Id: xen-devel@lists.xenproject.org --------------010306040504080802050805 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit On 12/02/16 14:59, Andrew Cooper wrote: > Coverity correctly identifies that the changes in mtrr_attrib_to_str() > introduce dead code. strings[] is a 2d array, rather than an array of > strings, which means that strings[x] will never be a NULL pointer. > > Adjust the check to compenstate, by looking for a NUL in strings[x][0] > instead. > > Curiously, Coverity did not notice the same error with memory_type_to_str(). > There was also a further error; the strings were not NULL terminated, which > made the return type of memory_type_to_str() erronious. > > Bump the 2D array to 3 characters, so the strings retain their NUL characters, > and introduce an ASSERT() as requested on one thread of the original patch. > > Signed-off-by: Andrew Cooper What about something like this instead? (Ported to be on top of this patch, since it's already been committed.) -George [PATCH] xen/p2m: Make dump table printing less clever Rather than detecting whether to print out the numerical value of the memory type based on whether the second byte of the stringified value is a null character, just always print out both. Signed-off-by: George Dunlap --- CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan --- xen/arch/x86/mm/p2m-ept.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 3cb6868..be528e7 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -1204,7 +1204,7 @@ void ept_p2m_uninit(struct p2m_domain *p2m) static const char *memory_type_to_str(unsigned int x) { - static const char memory_types[8][3] = { + static const char *memory_types[8] = { [MTRR_TYPE_UNCACHABLE] = "UC", [MTRR_TYPE_WRCOMB] = "WC", [MTRR_TYPE_WRTHROUGH] = "WT", @@ -1262,14 +1262,13 @@ static void ept_dump_p2m_table(unsigned char key) if ( ept_entry->sa_p2mt == p2m_populate_on_demand ) printk("gfn: %13lx order: %2d PoD\n", gfn, order); else - printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %c%c%c\n", + printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %s(%d)%c\n", gfn, order, ept_entry->mfn + 0UL, ept_entry->r ? 'r' : ' ', ept_entry->w ? 'w' : ' ', ept_entry->x ? 'x' : ' ', - memory_type_to_str(ept_entry->emt)[0], - memory_type_to_str(ept_entry->emt)[1] - ?: ept_entry->emt + '0', + memory_type_to_str(ept_entry->emt), + ept_entry->emt, c ?: ept_entry->ipat ? '!' : ' '); if ( !(record_counter++ % 100) ) -- 2.1.4 --------------010306040504080802050805 Content-Type: text/x-patch; name="0001-xen-p2m-Make-dump-table-printing-less-clever.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-xen-p2m-Make-dump-table-printing-less-clever.patch" >>From b9d1d77c001507c4c414b83666a3f59e327364c3 Mon Sep 17 00:00:00 2001 From: George Dunlap Date: Mon, 15 Feb 2016 14:58:29 +0000 Subject: [PATCH] xen/p2m: Make dump table printing less clever Rather than detecting whether to print out the numerical value of the memory type based on whether the second byte of the stringified value is a null character, just always print out both. Signed-off-by: George Dunlap --- CC: Andrew Cooper CC: Jan Beulich CC: Tim Deegan --- xen/arch/x86/mm/p2m-ept.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 3cb6868..be528e7 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -1204,7 +1204,7 @@ void ept_p2m_uninit(struct p2m_domain *p2m) static const char *memory_type_to_str(unsigned int x) { - static const char memory_types[8][3] = { + static const char *memory_types[8] = { [MTRR_TYPE_UNCACHABLE] = "UC", [MTRR_TYPE_WRCOMB] = "WC", [MTRR_TYPE_WRTHROUGH] = "WT", @@ -1262,14 +1262,13 @@ static void ept_dump_p2m_table(unsigned char key) if ( ept_entry->sa_p2mt == p2m_populate_on_demand ) printk("gfn: %13lx order: %2d PoD\n", gfn, order); else - printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %c%c%c\n", + printk("gfn: %13lx order: %2d mfn: %13lx %c%c%c %s(%d)%c\n", gfn, order, ept_entry->mfn + 0UL, ept_entry->r ? 'r' : ' ', ept_entry->w ? 'w' : ' ', ept_entry->x ? 'x' : ' ', - memory_type_to_str(ept_entry->emt)[0], - memory_type_to_str(ept_entry->emt)[1] - ?: ept_entry->emt + '0', + memory_type_to_str(ept_entry->emt), + ept_entry->emt, c ?: ept_entry->ipat ? '!' : ' '); if ( !(record_counter++ % 100) ) -- 2.1.4 --------------010306040504080802050805 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------010306040504080802050805--