[kvm-unit-tests PATCH 0/4] add fmt warnings to printf, and fix bad uses

[kvm-unit-tests PATCH 0/4] add fmt warnings to printf, and fix bad uses

[Andrew Jones]

Thomas suggested adding __attribute__((format(printf, i, j))) to
our *printf functions. So I did. And then fixed all the warnings...

Andrew Jones (4):
  lib: *printf: warn on format/arg mismatch
  lib/alloc: fix format warnings, add/use PRIx64
  arm64: fix printf format warnings
  x86: fix printf format warnings

 lib/alloc.c               | 13 +++++++------
 lib/arm64/processor.c     |  8 ++++----
 lib/libcflat.h            | 24 +++++++++++++++++++-----
 lib/x86/desc.c            |  6 +++---
 lib/x86/vm.c              |  6 +++---
 x86/access.c              |  4 ++--
 x86/eventinj.c            |  8 ++++----
 x86/kvmclock.c            |  2 +-
 x86/kvmclock_test.c       |  6 +++---
 x86/msr.c                 |  2 +-
 x86/s3.c                  |  6 +++---
 x86/svm.c                 | 12 ++++++------
 x86/taskswitch.c          |  2 +-
 x86/taskswitch2.c         |  2 +-
 x86/tsc.c                 |  4 ++--
 x86/tscdeadline_latency.c |  2 +-
 x86/vmx.c                 | 18 +++++++++---------
 x86/vmx_tests.c           | 20 ++++++++++----------
 x86/xsave.c               |  2 +-
 19 files changed, 81 insertions(+), 66 deletions(-)

-- 
2.4.3

[kvm-unit-tests PATCH 1/4] lib: *printf: warn on format/arg mismatch

[Andrew Jones]

Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/libcflat.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/libcflat.h b/lib/libcflat.h
index 39100f29713b8..ce09d34e658b3 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -51,13 +51,15 @@ typedef _Bool		bool;
 extern void puts(const char *s);
 extern void exit(int code);
 extern void abort(void);
-
-extern int printf(const char *fmt, ...);
-extern int snprintf(char *buf, int size, const char *fmt, ...);
-extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
-extern int vprintf(const char *fmt, va_list va);
 extern long atol(const char *ptr);
 
+#define _f(i,j) __attribute__((format(printf, i, j)))
+extern int printf(const char *fmt, ...) _f(1,2);
+extern int snprintf(char *buf, int size, const char *fmt, ...) _f(3,4);
+extern int vsnprintf(char *buf, int size, const char *fmt, va_list va) _f(3,0);
+extern int vprintf(const char *fmt, va_list va) _f(1,0);
+#undef _f
+
 extern void report_prefix_push(const char *prefix);
 extern void report_prefix_pop(void);
 extern void report(const char *msg_fmt, bool pass, ...);
-- 
2.4.3

[kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/alloc.c    | 13 +++++++------
 lib/libcflat.h | 12 ++++++++++++
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/alloc.c b/lib/alloc.c
index 34f71a337d868..e345843b7fe53 100644
--- a/lib/alloc.c
+++ b/lib/alloc.c
@@ -28,13 +28,13 @@ void phys_alloc_show(void)
 	int i;
 
 	spin_lock(&lock);
-	printf("phys_alloc minimum alignment: 0x%llx\n", align_min);
+	printf("phys_alloc minimum alignment: 0x%" PRIx64 "\n", align_min);
 	for (i = 0; i < nr_regions; ++i)
-		printf("%016llx-%016llx [%s]\n",
+		printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n",
 			regions[i].base,
 			regions[i].base + regions[i].size - 1,
 			"USED");
-	printf("%016llx-%016llx [%s]\n", base, top - 1, "FREE");
+	printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", base, top - 1, "FREE");
 	spin_unlock(&lock);
 }
 
@@ -76,9 +76,10 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
 	size += addr - base;
 
 	if ((top_safe - base) < size) {
-		printf("phys_alloc: requested=0x%llx (align=0x%llx), "
-		       "need=0x%llx, but free=0x%llx. "
-		       "top=0x%llx, top_safe=0x%llx\n",
+		printf("phys_alloc: requested=0x%" PRIx64
+		       " (align=0x%" PRIx64 "), "
+		       "need=0x%" PRIx64 ", but free=0x%" PRIx64 ". "
+		       "top=0x%" PRIx64 ", top_safe=0x%" PRIx64 "\n",
 		       size_orig, align, size, top_safe - base,
 		       top, top_safe);
 		spin_unlock(&lock);
diff --git a/lib/libcflat.h b/lib/libcflat.h
index ce09d34e658b3..36e300bf4d0ff 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -48,6 +48,18 @@ typedef _Bool		bool;
 #define false 0
 #define true  1
 
+#if __SIZEOF_LONG__ == 8
+#  define __PRI64_PREFIX	"l"
+#  define __PRIPTR_PREFIX	"l"
+#else
+#  define __PRI64_PREFIX	"ll"
+#  define __PRIPTR_PREFIX
+#endif
+#define PRId64  __PRI64_PREFIX	"d"
+#define PRIu64  __PRI64_PREFIX	"u"
+#define PRIx64  __PRI64_PREFIX	"x"
+#define PRIxPTR __PRIPTR_PREFIX	"x"
+
 extern void puts(const char *s);
 extern void exit(int code);
 extern void abort(void);
-- 
2.4.3

[kvm-unit-tests PATCH 3/4] arm64: fix printf format warnings

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/arm64/processor.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index c553390c969ea..deeab4ec9c8ac 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -66,12 +66,12 @@ void show_regs(struct pt_regs *regs)
 {
 	int i;
 
-	printf("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
+	printf("pc : [<%016lx>] lr : [<%016lx>] pstate: %08lx\n",
 			regs->pc, regs->regs[30], regs->pstate);
-	printf("sp : %016llx\n", regs->sp);
+	printf("sp : %016lx\n", regs->sp);
 
 	for (i = 29; i >= 0; --i) {
-		printf("x%-2d: %016llx ", i, regs->regs[i]);
+		printf("x%-2d: %016lx ", i, regs->regs[i]);
 		if (i % 2 == 0)
 			printf("\n");
 	}
@@ -121,7 +121,7 @@ static void bad_exception(enum vector v, struct pt_regs *regs,
 	}
 
 	printf("Vector: %d (%s)\n", v, vector_names[v]);
-	printf("ESR_EL1: %8s%08lx, ec=0x%x (%s)\n", "", esr, ec, ec_names[ec]);
+	printf("ESR_EL1: %8s%08x, ec=0x%x (%s)\n", "", esr, ec, ec_names[ec]);
 	printf("FAR_EL1: %016lx (%svalid)\n", far, far_valid ? "" : "not ");
 	printf("Exception frame registers:\n");
 	show_regs(regs);
-- 
2.4.3

[kvm-unit-tests PATCH 4/4] x86: fix printf format warnings

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/x86/desc.c            |  6 +++---
 lib/x86/vm.c              |  6 +++---
 x86/access.c              |  4 ++--
 x86/eventinj.c            |  8 ++++----
 x86/kvmclock.c            |  2 +-
 x86/kvmclock_test.c       |  6 +++---
 x86/msr.c                 |  2 +-
 x86/s3.c                  |  6 +++---
 x86/svm.c                 | 12 ++++++------
 x86/taskswitch.c          |  2 +-
 x86/taskswitch2.c         |  2 +-
 x86/tsc.c                 |  4 ++--
 x86/tscdeadline_latency.c |  2 +-
 x86/vmx.c                 | 18 +++++++++---------
 x86/vmx_tests.c           | 20 ++++++++++----------
 x86/xsave.c               |  2 +-
 16 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 1a80c01283853..0a79a41b56cb1 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -53,7 +53,7 @@ static void check_exception_table(struct ex_regs *regs)
             return;
         }
     }
-    printf("unhandled exception %d\n", regs->vector);
+    printf("unhandled exception %ld\n", regs->vector);
     exit(7);
 }
 
@@ -75,9 +75,9 @@ void do_handle_exception(struct ex_regs *regs)
 		exception_handlers[regs->vector](regs);
 		return;
 	}
-	printf("unhandled cpu exception %d\n", regs->vector);
+	printf("unhandled cpu exception %ld\n", regs->vector);
 	if (regs->vector == 14)
-		printf("PF at %p addr %p\n", regs->rip, read_cr2());
+		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
 	exit(7);
 }
 
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index b820c7def72ab..7ce7bbc03eef9 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -161,9 +161,9 @@ static void setup_mmu(unsigned long len)
     write_cr0(X86_CR0_PG |X86_CR0_PE | X86_CR0_WP);
 
     printf("paging enabled\n");
-    printf("cr0 = %x\n", read_cr0());
-    printf("cr3 = %x\n", read_cr3());
-    printf("cr4 = %x\n", read_cr4());
+    printf("cr0 = %lx\n", read_cr0());
+    printf("cr3 = %lx\n", read_cr3());
+    printf("cr4 = %lx\n", read_cr4());
 }
 
 void setup_vm()
diff --git a/x86/access.c b/x86/access.c
index 02b20ca8d5c96..c2d8db1a06e35 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -455,13 +455,13 @@ static void dump_mapping(ac_test_t *at)
 	unsigned long root = read_cr3();
 	int i;
 
-	printf("Dump mapping: address: %llx\n", at->virt);
+	printf("Dump mapping: address: %p\n", at->virt);
 	for (i = 4; i >= 1 && (i >= 2 || !at->flags[AC_PDE_PSE]); --i) {
 		pt_element_t *vroot = va(root & PT_BASE_ADDR_MASK);
 		unsigned index = PT_INDEX((unsigned long)at->virt, i);
 		pt_element_t pte = vroot[index];
 
-		printf("------L%d: %llx\n", i, pte);
+		printf("------L%d: %lx\n", i, pte);
 		root = vroot[index];
 	}
 }
diff --git a/x86/eventinj.c b/x86/eventinj.c
index bddedce3797c2..ac8653f3b2030 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -90,7 +90,7 @@ static void of_isr(struct ex_regs *r)
 
 static void np_isr(struct ex_regs *r)
 {
-	printf("NP isr running %x err=%x\n", r->rip, r->error_code);
+	printf("NP isr running %lx err=%lx\n", r->rip, r->error_code);
 	set_idt_sel(33, read_cs());
 	test_count++;
 }
@@ -109,14 +109,14 @@ static void bp_isr(struct ex_regs *r)
 
 static void nested_nmi_isr(struct ex_regs *r)
 {
-	printf("Nested NMI isr running rip=%x\n", r->rip);
+	printf("Nested NMI isr running rip=%lx\n", r->rip);
 
 	if (r->rip != (ulong)&isr_iret_ip)
 		test_count++;
 }
 static void nmi_isr(struct ex_regs *r)
 {
-	printf("NMI isr running %x\n", &isr_iret_ip);
+	printf("NMI isr running %p\n", &isr_iret_ip);
 	test_count++;
 	handle_exception(2, nested_nmi_isr);
 	printf("Sending nested NMI to self\n");
@@ -129,7 +129,7 @@ unsigned long *iret_stack;
 
 static void nested_nmi_iret_isr(struct ex_regs *r)
 {
-	printf("Nested NMI isr running rip=%x\n", r->rip);
+	printf("Nested NMI isr running rip=%lx\n", r->rip);
 
 	if (r->rip == iret_stack[-3])
 		test_count++;
diff --git a/x86/kvmclock.c b/x86/kvmclock.c
index 5b831c5673910..588d8ece676d7 100644
--- a/x86/kvmclock.c
+++ b/x86/kvmclock.c
@@ -228,7 +228,7 @@ void kvm_clock_init(void *data)
         int index = smp_id();
         struct pvclock_vcpu_time_info *hvc = &hv_clock[index];
 
-        printf("kvm-clock: cpu %d, msr 0x:%lx \n", index, hvc);
+        printf("kvm-clock: cpu %d, msr %p\n", index, hvc);
         wrmsr(MSR_KVM_SYSTEM_TIME, (unsigned long)hvc | 1);
 }
 
diff --git a/x86/kvmclock_test.c b/x86/kvmclock_test.c
index b6da5ebbe1097..c8ffa126803d5 100644
--- a/x86/kvmclock_test.c
+++ b/x86/kvmclock_test.c
@@ -67,7 +67,7 @@ static void kvm_clock_test(void *data)
                         ++hv_test_info->warps;
                         if (delta < hv_test_info->worst){
                                 hv_test_info->worst = delta;
-                                printf("Worst warp %lld %\n", hv_test_info->worst);
+                                printf("Worst warp %lld\n", hv_test_info->worst);
                         }
                         spin_unlock(&hv_test_info->lock);
                 }
@@ -102,8 +102,8 @@ static int cycle_test(int ncpus, int check, struct test_info *ti)
         printf("Total vcpus: %d\n", ncpus);
         printf("Test  loops: %ld\n", loops);
         if (check == 1) {
-                printf("Total warps:  %lld\n", ti->warps);
-                printf("Total stalls: %lld\n", ti->stalls);
+                printf("Total warps:  %" PRId64 "\n", ti->warps);
+                printf("Total stalls: %" PRId64 "\n", ti->stalls);
                 printf("Worst warp:   %lld\n", ti->worst);
         } else
                 printf("TSC cycles:  %lld\n", end - begin);
diff --git a/x86/msr.c b/x86/msr.c
index ec4710ed478b9..50b13840ed946 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -88,7 +88,7 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l
     wrmsr(msr_index, input);
     r = rdmsr(msr_index);
     if (expected != r) {
-        printf("testing %s: output = 0x%x:0x%x expected = 0x%x:0x%x\n", sptr, r >> 32, r, expected >> 32, expected);
+        printf("testing %s: output = 0x%llx:0x%llx expected = 0x%llx:0x%llx\n", sptr, r >> 32, r, expected >> 32, expected);
     }
     report(sptr, expected == r);
 }
diff --git a/x86/s3.c b/x86/s3.c
index 8bf71da4e70b1..a3d76a3de39bd 100644
--- a/x86/s3.c
+++ b/x86/s3.c
@@ -6,7 +6,7 @@ u32* find_resume_vector_addr(void)
     struct facs_descriptor_rev1 *facs = find_acpi_table_addr(FACS_SIGNATURE);
     if (!facs)
         return 0;
-    printf("FACS is at %x\n", facs);
+    printf("FACS is at %p\n", facs);
     return &facs->firmware_waking_vector;
 }
 
@@ -46,10 +46,10 @@ int main(int argc, char **argv)
 
 	*resume_vector_ptr = (u32)(ulong)resume_vec;
 
-	printf("resume vector addr is %x\n", resume_vector_ptr);
+	printf("resume vector addr is %p\n", resume_vector_ptr);
 	for (addr = &resume_start; addr < &resume_end; addr++)
 		*resume_vec++ = *addr;
-	printf("copy resume code from %x\n", &resume_start);
+	printf("copy resume code from %p\n", &resume_start);
 
 	/* Setup RTC alarm to wake up on the next second.  */
 	while ((rtc_in(RTC_REG_A) & REG_A_UIP) == 0);
diff --git a/x86/svm.c b/x86/svm.c
index 1046ddf73732f..934b2ae91fa81 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -934,9 +934,9 @@ static bool latency_finished(struct test *test)
 
 static bool latency_check(struct test *test)
 {
-    printf("    Latency VMRUN : max: %d min: %d avg: %d\n", latvmrun_max,
+    printf("    Latency VMRUN : max: %ld min: %ld avg: %ld\n", latvmrun_max,
             latvmrun_min, vmrun_sum / LATENCY_RUNS);
-    printf("    Latency VMEXIT: max: %d min: %d avg: %d\n", latvmexit_max,
+    printf("    Latency VMEXIT: max: %ld min: %ld avg: %ld\n", latvmexit_max,
             latvmexit_min, vmexit_sum / LATENCY_RUNS);
     return true;
 }
@@ -998,13 +998,13 @@ static bool lat_svm_insn_finished(struct test *test)
 
 static bool lat_svm_insn_check(struct test *test)
 {
-    printf("    Latency VMLOAD: max: %d min: %d avg: %d\n", latvmload_max,
+    printf("    Latency VMLOAD: max: %ld min: %ld avg: %ld\n", latvmload_max,
             latvmload_min, vmload_sum / LATENCY_RUNS);
-    printf("    Latency VMSAVE: max: %d min: %d avg: %d\n", latvmsave_max,
+    printf("    Latency VMSAVE: max: %ld min: %ld avg: %ld\n", latvmsave_max,
             latvmsave_min, vmsave_sum / LATENCY_RUNS);
-    printf("    Latency STGI:   max: %d min: %d avg: %d\n", latstgi_max,
+    printf("    Latency STGI:   max: %ld min: %ld avg: %ld\n", latstgi_max,
             latstgi_min, stgi_sum / LATENCY_RUNS);
-    printf("    Latency CLGI:   max: %d min: %d avg: %d\n", latclgi_max,
+    printf("    Latency CLGI:   max: %ld min: %ld avg: %ld\n", latclgi_max,
             latclgi_min, clgi_sum / LATENCY_RUNS);
     return true;
 }
diff --git a/x86/taskswitch.c b/x86/taskswitch.c
index 5e26f57acc2ec..01483a1643561 100644
--- a/x86/taskswitch.c
+++ b/x86/taskswitch.c
@@ -16,7 +16,7 @@ static __attribute__((used, regparm(1))) void
 fault_handler(unsigned long error_code)
 {
 	print_current_tss_info();
-	printf("error code %x\n", error_code);
+	printf("error code %lx\n", error_code);
 
 	tss.eip += 2;
 
diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
index db3e41aa82186..dbc9a2525d01a 100644
--- a/x86/taskswitch2.c
+++ b/x86/taskswitch2.c
@@ -62,7 +62,7 @@ start:
 
 void do_pf_tss(ulong *error_code)
 {
-	printf("PF task is running %x %x\n", error_code, *(ulong*)error_code);
+	printf("PF task is running %p %lx\n", error_code, *(ulong*)error_code);
 	print_current_tss_info();
 	if (*(ulong*)error_code == 0x2) /* write access, not present */
 		test_count++;
diff --git a/x86/tsc.c b/x86/tsc.c
index c71dc2a7abe09..6f89c911c2bc4 100644
--- a/x86/tsc.c
+++ b/x86/tsc.c
@@ -14,7 +14,7 @@ void test_wrtsc(u64 t1)
 
 	wrtsc(t1);
 	t2 = rdtsc();
-	printf("rdtsc after wrtsc(%lld): %lld\n", t1, t2);
+	printf("rdtsc after wrtsc(%" PRId64 "): %" PRId64 "\n", t1, t2);
 }
 
 void test_rdtscp(u64 aux)
@@ -32,7 +32,7 @@ int main()
 
 	t1 = rdtsc();
 	t2 = rdtsc();
-	printf("rdtsc latency %lld\n", (unsigned)(t2 - t1));
+	printf("rdtsc latency %u\n", (unsigned)(t2 - t1));
 
 	test_wrtsc(0);
 	test_wrtsc(100000000000ull);
diff --git a/x86/tscdeadline_latency.c b/x86/tscdeadline_latency.c
index 42f19cb92e518..8a3f3c6faf94f 100644
--- a/x86/tscdeadline_latency.c
+++ b/x86/tscdeadline_latency.c
@@ -129,7 +129,7 @@ int main(int argc, char **argv)
     for (i = 0; i < table_idx; i++) {
         if (hitmax && i == table_idx-1)
             printf("hit max: %d < ", breakmax);
-        printf("latency: %d\n", table[i]);
+        printf("latency: %" PRId64 "\n", table[i]);
     }
 
     return report_summary();
diff --git a/x86/vmx.c b/x86/vmx.c
index 3fa1a735881f7..e047a59ab1129 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -144,17 +144,17 @@ void print_vmexit_info()
 	guest_rip = vmcs_read(GUEST_RIP);
 	guest_rsp = vmcs_read(GUEST_RSP);
 	printf("VMEXIT info:\n");
-	printf("\tvmexit reason = %d\n", reason);
-	printf("\texit qualification = 0x%x\n", exit_qual);
-	printf("\tBit 31 of reason = %x\n", (vmcs_read(EXI_REASON) >> 31) & 1);
-	printf("\tguest_rip = 0x%llx\n", guest_rip);
-	printf("\tRAX=0x%llx    RBX=0x%llx    RCX=0x%llx    RDX=0x%llx\n",
+	printf("\tvmexit reason = %ld\n", reason);
+	printf("\texit qualification = 0x%lx\n", exit_qual);
+	printf("\tBit 31 of reason = %lx\n", (vmcs_read(EXI_REASON) >> 31) & 1);
+	printf("\tguest_rip = 0x%lx\n", guest_rip);
+	printf("\tRAX=0x%lx    RBX=0x%lx    RCX=0x%lx    RDX=0x%lx\n",
 		regs.rax, regs.rbx, regs.rcx, regs.rdx);
-	printf("\tRSP=0x%llx    RBP=0x%llx    RSI=0x%llx    RDI=0x%llx\n",
+	printf("\tRSP=0x%lx    RBP=0x%lx    RSI=0x%lx    RDI=0x%lx\n",
 		guest_rsp, regs.rbp, regs.rsi, regs.rdi);
-	printf("\tR8 =0x%llx    R9 =0x%llx    R10=0x%llx    R11=0x%llx\n",
+	printf("\tR8 =0x%lx    R9 =0x%lx    R10=0x%lx    R11=0x%lx\n",
 		regs.r8, regs.r9, regs.r10, regs.r11);
-	printf("\tR12=0x%llx    R13=0x%llx    R14=0x%llx    R15=0x%llx\n",
+	printf("\tR12=0x%lx    R13=0x%lx    R14=0x%lx    R15=0x%lx\n",
 		regs.r12, regs.r13, regs.r14, regs.r15);
 }
 
@@ -842,7 +842,7 @@ static int handle_hypercall()
 	case HYPERCALL_VMEXIT:
 		return VMX_TEST_VMEXIT;
 	default:
-		printf("ERROR : Invalid hypercall number : %d\n", hypercall_no);
+		printf("ERROR : Invalid hypercall number : %ld\n", hypercall_no);
 	}
 	return VMX_TEST_EXIT;
 }
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bb65c06058ff8..fafac1c79969f 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -211,7 +211,7 @@ int preemption_timer_exit_handler()
 		}
 		break;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 	vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
@@ -301,7 +301,7 @@ static int test_ctrl_pat_exit_handler()
 		vmcs_write(GUEST_RIP, guest_rip + 3);
 		return VMX_TEST_RESUME;
 	default:
-		printf("ERROR : Undefined exit reason, reason = %d.\n", reason);
+		printf("ERROR : Undefined exit reason, reason = %ld.\n", reason);
 		break;
 	}
 	return VMX_TEST_VMEXIT;
@@ -370,7 +370,7 @@ static int test_ctrl_efer_exit_handler()
 		vmcs_write(GUEST_RIP, guest_rip + 3);
 		return VMX_TEST_RESUME;
 	default:
-		printf("ERROR : Undefined exit reason, reason = %d.\n", reason);
+		printf("ERROR : Undefined exit reason, reason = %ld.\n", reason);
 		break;
 	}
 	return VMX_TEST_VMEXIT;
@@ -555,7 +555,7 @@ static int cr_shadowing_exit_handler()
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 	return VMX_TEST_VMEXIT;
@@ -715,8 +715,8 @@ static int iobmp_exit_handler()
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		printf("guest_rip = 0x%llx\n", guest_rip);
-		printf("\tERROR : Undefined exit reason, reason = %d.\n", reason);
+		printf("guest_rip = 0x%lx\n", guest_rip);
+		printf("\tERROR : Undefined exit reason, reason = %ld.\n", reason);
 		break;
 	}
 	return VMX_TEST_VMEXIT;
@@ -1144,7 +1144,7 @@ static int ept_exit_handler()
 		}
 		return VMX_TEST_RESUME;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 	return VMX_TEST_VMEXIT;
@@ -1206,7 +1206,7 @@ static int vpid_exit_handler()
 		vmcs_write(GUEST_RIP, guest_rip + insn_len);
 		return VMX_TEST_RESUME;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 	return VMX_TEST_VMEXIT;
@@ -1363,7 +1363,7 @@ static int interrupt_exit_handler(void)
 			vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
 		return VMX_TEST_RESUME;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 
@@ -1585,7 +1585,7 @@ static int vmmcall_exit_handler()
 		       (vmcs_read(EXI_INTR_INFO) & 0xff) == UD_VECTOR);
 		break;
 	default:
-		printf("Unknown exit reason, %d\n", reason);
+		printf("Unknown exit reason, %ld\n", reason);
 		print_vmexit_info();
 	}
 
diff --git a/x86/xsave.c b/x86/xsave.c
index e471835b42fd9..52142d2cdb93b 100644
--- a/x86/xsave.c
+++ b/x86/xsave.c
@@ -75,7 +75,7 @@ void test_xsave(void)
     printf("Legal instruction testing:\n");
 
     supported_xcr0 = get_supported_xcr0();
-    printf("Supported XCR0 bits: 0x%x\n", supported_xcr0);
+    printf("Supported XCR0 bits: 0x%lx\n", supported_xcr0);
 
     test_bits = XSTATE_FP | XSTATE_SSE;
     report("Check minimal XSAVE required bits",
-- 
2.4.3

Re: [kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Thomas Huth]

On 29.02.2016 21:19, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/alloc.c    | 13 +++++++------
>  lib/libcflat.h | 12 ++++++++++++
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/alloc.c b/lib/alloc.c
> index 34f71a337d868..e345843b7fe53 100644
> --- a/lib/alloc.c
> +++ b/lib/alloc.c
> @@ -28,13 +28,13 @@ void phys_alloc_show(void)
>  	int i;
>  
>  	spin_lock(&lock);
> -	printf("phys_alloc minimum alignment: 0x%llx\n", align_min);
> +	printf("phys_alloc minimum alignment: 0x%" PRIx64 "\n", align_min);
>  	for (i = 0; i < nr_regions; ++i)
> -		printf("%016llx-%016llx [%s]\n",
> +		printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n",
>  			regions[i].base,
>  			regions[i].base + regions[i].size - 1,
>  			"USED");
> -	printf("%016llx-%016llx [%s]\n", base, top - 1, "FREE");
> +	printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", base, top - 1, "FREE");
>  	spin_unlock(&lock);
>  }
>  
> @@ -76,9 +76,10 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
>  	size += addr - base;
>  
>  	if ((top_safe - base) < size) {
> -		printf("phys_alloc: requested=0x%llx (align=0x%llx), "
> -		       "need=0x%llx, but free=0x%llx. "
> -		       "top=0x%llx, top_safe=0x%llx\n",
> +		printf("phys_alloc: requested=0x%" PRIx64
> +		       " (align=0x%" PRIx64 "), "
> +		       "need=0x%" PRIx64 ", but free=0x%" PRIx64 ". "
> +		       "top=0x%" PRIx64 ", top_safe=0x%" PRIx64 "\n",
>  		       size_orig, align, size, top_safe - base,
>  		       top, top_safe);
>  		spin_unlock(&lock);
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index ce09d34e658b3..36e300bf4d0ff 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -48,6 +48,18 @@ typedef _Bool		bool;
>  #define false 0
>  #define true  1
>  
> +#if __SIZEOF_LONG__ == 8
> +#  define __PRI64_PREFIX	"l"
> +#  define __PRIPTR_PREFIX	"l"
> +#else
> +#  define __PRI64_PREFIX	"ll"
> +#  define __PRIPTR_PREFIX
> +#endif
> +#define PRId64  __PRI64_PREFIX	"d"
> +#define PRIu64  __PRI64_PREFIX	"u"
> +#define PRIx64  __PRI64_PREFIX	"x"
> +#define PRIxPTR __PRIPTR_PREFIX	"x"

So libcflat uses the header stdint.h from the standard gcc installation
to get the uint64_t and friends, but you define the PRIx64 etc. here
manually? That sounds strange... You could simply include "inttypes.h"
instead of "stdint.h" in this libcflat.h header file, then you would get
PRIx64 and friends automatically instead.

OTOH, for a stand-alone binary like kvm-unit-tests, it's also somewhat
risky to compile without "-nostdinc" since unwanted headers might be
included that way (e.g. what happens if someone writes "#include
<stdio.h>" by accident? You likely get some nice confusion...). So maybe
it would be better to compile with "-nostdinc" instead and define all
necessary things manually here?

 Thomas

Re: [kvm-unit-tests PATCH 3/4] arm64: fix printf format warnings

[Thomas Huth]

On 29.02.2016 21:19, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/arm64/processor.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
> index c553390c969ea..deeab4ec9c8ac 100644
> --- a/lib/arm64/processor.c
> +++ b/lib/arm64/processor.c
> @@ -66,12 +66,12 @@ void show_regs(struct pt_regs *regs)
>  {
>  	int i;
>  
> -	printf("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
> +	printf("pc : [<%016lx>] lr : [<%016lx>] pstate: %08lx\n",
>  			regs->pc, regs->regs[30], regs->pstate);
> -	printf("sp : %016llx\n", regs->sp);
> +	printf("sp : %016lx\n", regs->sp);
>  
>  	for (i = 29; i >= 0; --i) {
> -		printf("x%-2d: %016llx ", i, regs->regs[i]);
> +		printf("x%-2d: %016lx ", i, regs->regs[i]);
>  		if (i % 2 == 0)
>  			printf("\n");
>  	}

Why don't you use PRIx64 here? The regs are defined as "u64", not as
"long", so PRIx64 should fit better, shouldn't it?

 Thomas

Re: [kvm-unit-tests PATCH 4/4] x86: fix printf format warnings

[Thomas Huth]

On 29.02.2016 21:19, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/x86/desc.c            |  6 +++---
>  lib/x86/vm.c              |  6 +++---
>  x86/access.c              |  4 ++--
>  x86/eventinj.c            |  8 ++++----
>  x86/kvmclock.c            |  2 +-
>  x86/kvmclock_test.c       |  6 +++---
>  x86/msr.c                 |  2 +-
>  x86/s3.c                  |  6 +++---
>  x86/svm.c                 | 12 ++++++------
>  x86/taskswitch.c          |  2 +-
>  x86/taskswitch2.c         |  2 +-
>  x86/tsc.c                 |  4 ++--
>  x86/tscdeadline_latency.c |  2 +-
>  x86/vmx.c                 | 18 +++++++++---------
>  x86/vmx_tests.c           | 20 ++++++++++----------
>  x86/xsave.c               |  2 +-
>  16 files changed, 51 insertions(+), 51 deletions(-)
> 
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 1a80c01283853..0a79a41b56cb1 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -53,7 +53,7 @@ static void check_exception_table(struct ex_regs *regs)
>              return;
>          }
>      }
> -    printf("unhandled exception %d\n", regs->vector);
> +    printf("unhandled exception %ld\n", regs->vector);

That could also be %lu instead, I think, but it likely does not really
matter here.

>      exit(7);
>  }
>  
> @@ -75,9 +75,9 @@ void do_handle_exception(struct ex_regs *regs)
>  		exception_handlers[regs->vector](regs);
>  		return;
>  	}
> -	printf("unhandled cpu exception %d\n", regs->vector);
> +	printf("unhandled cpu exception %ld\n", regs->vector);
>  	if (regs->vector == 14)
> -		printf("PF at %p addr %p\n", regs->rip, read_cr2());
> +		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
>  	exit(7);
>  }
>  
...
> diff --git a/x86/kvmclock_test.c b/x86/kvmclock_test.c
> index b6da5ebbe1097..c8ffa126803d5 100644
> --- a/x86/kvmclock_test.c
> +++ b/x86/kvmclock_test.c
> @@ -67,7 +67,7 @@ static void kvm_clock_test(void *data)
>                          ++hv_test_info->warps;
>                          if (delta < hv_test_info->worst){
>                                  hv_test_info->worst = delta;
> -                                printf("Worst warp %lld %\n", hv_test_info->worst);
> +                                printf("Worst warp %lld\n", hv_test_info->worst);

Ah, nice, so this addition of __attribute__((format)) indeed revealed
also some real problematic format strings :-)

>                          }
>                          spin_unlock(&hv_test_info->lock);
>                  }
> @@ -102,8 +102,8 @@ static int cycle_test(int ncpus, int check, struct test_info *ti)
>          printf("Total vcpus: %d\n", ncpus);
>          printf("Test  loops: %ld\n", loops);
>          if (check == 1) {
> -                printf("Total warps:  %lld\n", ti->warps);
> -                printf("Total stalls: %lld\n", ti->stalls);
> +                printf("Total warps:  %" PRId64 "\n", ti->warps);
> +                printf("Total stalls: %" PRId64 "\n", ti->stalls);
>                  printf("Worst warp:   %lld\n", ti->worst);
>          } else
>                  printf("TSC cycles:  %lld\n", end - begin);
> diff --git a/x86/msr.c b/x86/msr.c
> index ec4710ed478b9..50b13840ed946 100644
> --- a/x86/msr.c
> +++ b/x86/msr.c
> @@ -88,7 +88,7 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l
>      wrmsr(msr_index, input);
>      r = rdmsr(msr_index);
>      if (expected != r) {
> -        printf("testing %s: output = 0x%x:0x%x expected = 0x%x:0x%x\n", sptr, r >> 32, r, expected >> 32, expected);
> +        printf("testing %s: output = 0x%llx:0x%llx expected = 0x%llx:0x%llx\n", sptr, r >> 32, r, expected >> 32, expected);

That code looks wrong (before and after your change, but in different
ways ;-)):
r is long long, so let's assume its value is 0x1234567890ABCDEF. You
would then get the following output:

 output = 0x12345678:0x1234567890ABCDEF

But the author likely intended:

 output = 0x12345678:0x90ABCDEF

Same for the "expected" value.

So either the lower part has explicitely be casted to (uint32_t), or
simply print the whole value only once with llx, without the colon
inbetween.

>      }
>      report(sptr, expected == r);
>  }
...

> diff --git a/x86/svm.c b/x86/svm.c
> index 1046ddf73732f..934b2ae91fa81 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -934,9 +934,9 @@ static bool latency_finished(struct test *test)
>  
>  static bool latency_check(struct test *test)
>  {
> -    printf("    Latency VMRUN : max: %d min: %d avg: %d\n", latvmrun_max,
> +    printf("    Latency VMRUN : max: %ld min: %ld avg: %ld\n", latvmrun_max,
>              latvmrun_min, vmrun_sum / LATENCY_RUNS);
> -    printf("    Latency VMEXIT: max: %d min: %d avg: %d\n", latvmexit_max,
> +    printf("    Latency VMEXIT: max: %ld min: %ld avg: %ld\n", latvmexit_max,
>              latvmexit_min, vmexit_sum / LATENCY_RUNS);
>      return true;
>  }
> @@ -998,13 +998,13 @@ static bool lat_svm_insn_finished(struct test *test)
>  
>  static bool lat_svm_insn_check(struct test *test)
>  {
> -    printf("    Latency VMLOAD: max: %d min: %d avg: %d\n", latvmload_max,
> +    printf("    Latency VMLOAD: max: %ld min: %ld avg: %ld\n", latvmload_max,
>              latvmload_min, vmload_sum / LATENCY_RUNS);
> -    printf("    Latency VMSAVE: max: %d min: %d avg: %d\n", latvmsave_max,
> +    printf("    Latency VMSAVE: max: %ld min: %ld avg: %ld\n", latvmsave_max,
>              latvmsave_min, vmsave_sum / LATENCY_RUNS);
> -    printf("    Latency STGI:   max: %d min: %d avg: %d\n", latstgi_max,
> +    printf("    Latency STGI:   max: %ld min: %ld avg: %ld\n", latstgi_max,
>              latstgi_min, stgi_sum / LATENCY_RUNS);
> -    printf("    Latency CLGI:   max: %d min: %d avg: %d\n", latclgi_max,
> +    printf("    Latency CLGI:   max: %ld min: %ld avg: %ld\n", latclgi_max,
>              latclgi_min, clgi_sum / LATENCY_RUNS);
>      return true;
>  }

The latxxx_min/max variables seem to be declared with u64 ... so maybe
better use PRIu64 here instead?

> diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
> index db3e41aa82186..dbc9a2525d01a 100644
> --- a/x86/taskswitch2.c
> +++ b/x86/taskswitch2.c
> @@ -62,7 +62,7 @@ start:
>  
>  void do_pf_tss(ulong *error_code)
>  {
> -	printf("PF task is running %x %x\n", error_code, *(ulong*)error_code);
> +	printf("PF task is running %p %lx\n", error_code, *(ulong*)error_code);

While you're at it, you could remove the (ulong*) typecast here, too.

>  	print_current_tss_info();
>  	if (*(ulong*)error_code == 0x2) /* write access, not present */
>  		test_count++;
...
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 3fa1a735881f7..e047a59ab1129 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -144,17 +144,17 @@ void print_vmexit_info()
>  	guest_rip = vmcs_read(GUEST_RIP);
>  	guest_rsp = vmcs_read(GUEST_RSP);
>  	printf("VMEXIT info:\n");
> -	printf("\tvmexit reason = %d\n", reason);
> -	printf("\texit qualification = 0x%x\n", exit_qual);
> -	printf("\tBit 31 of reason = %x\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> -	printf("\tguest_rip = 0x%llx\n", guest_rip);
> -	printf("\tRAX=0x%llx    RBX=0x%llx    RCX=0x%llx    RDX=0x%llx\n",
> +	printf("\tvmexit reason = %ld\n", reason);
> +	printf("\texit qualification = 0x%lx\n", exit_qual);
> +	printf("\tBit 31 of reason = %lx\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> +	printf("\tguest_rip = 0x%lx\n", guest_rip);
> +	printf("\tRAX=0x%lx    RBX=0x%lx    RCX=0x%lx    RDX=0x%lx\n",
>  		regs.rax, regs.rbx, regs.rcx, regs.rdx);
> -	printf("\tRSP=0x%llx    RBP=0x%llx    RSI=0x%llx    RDI=0x%llx\n",
> +	printf("\tRSP=0x%lx    RBP=0x%lx    RSI=0x%lx    RDI=0x%lx\n",
>  		guest_rsp, regs.rbp, regs.rsi, regs.rdi);
> -	printf("\tR8 =0x%llx    R9 =0x%llx    R10=0x%llx    R11=0x%llx\n",
> +	printf("\tR8 =0x%lx    R9 =0x%lx    R10=0x%lx    R11=0x%lx\n",
>  		regs.r8, regs.r9, regs.r10, regs.r11);
> -	printf("\tR12=0x%llx    R13=0x%llx    R14=0x%llx    R15=0x%llx\n",
> +	printf("\tR12=0x%lx    R13=0x%lx    R14=0x%lx    R15=0x%lx\n",
>  		regs.r12, regs.r13, regs.r14, regs.r15);
>  }

The registers are declared as u64 ... so use PRIx64 for them instead?

> @@ -842,7 +842,7 @@ static int handle_hypercall()
>  	case HYPERCALL_VMEXIT:
>  		return VMX_TEST_VMEXIT;
>  	default:
> -		printf("ERROR : Invalid hypercall number : %d\n", hypercall_no);
> +		printf("ERROR : Invalid hypercall number : %ld\n", hypercall_no);
>  	}
>  	return VMX_TEST_EXIT;
>  }
...
> diff --git a/x86/xsave.c b/x86/xsave.c
> index e471835b42fd9..52142d2cdb93b 100644
> --- a/x86/xsave.c
> +++ b/x86/xsave.c
> @@ -75,7 +75,7 @@ void test_xsave(void)
>      printf("Legal instruction testing:\n");
>  
>      supported_xcr0 = get_supported_xcr0();
> -    printf("Supported XCR0 bits: 0x%x\n", supported_xcr0);
> +    printf("Supported XCR0 bits: 0x%lx\n", supported_xcr0);

Also PRIx64 here?

 Thomas

Re: [kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Andrew Jones]

On Tue, Mar 01, 2016 at 05:58:52AM +0100, Thomas Huth wrote:
> On 29.02.2016 21:19, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/alloc.c    | 13 +++++++------
> >  lib/libcflat.h | 12 ++++++++++++
> >  2 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/alloc.c b/lib/alloc.c
> > index 34f71a337d868..e345843b7fe53 100644
> > --- a/lib/alloc.c
> > +++ b/lib/alloc.c
> > @@ -28,13 +28,13 @@ void phys_alloc_show(void)
> >  	int i;
> >  
> >  	spin_lock(&lock);
> > -	printf("phys_alloc minimum alignment: 0x%llx\n", align_min);
> > +	printf("phys_alloc minimum alignment: 0x%" PRIx64 "\n", align_min);
> >  	for (i = 0; i < nr_regions; ++i)
> > -		printf("%016llx-%016llx [%s]\n",
> > +		printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n",
> >  			regions[i].base,
> >  			regions[i].base + regions[i].size - 1,
> >  			"USED");
> > -	printf("%016llx-%016llx [%s]\n", base, top - 1, "FREE");
> > +	printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", base, top - 1, "FREE");
> >  	spin_unlock(&lock);
> >  }
> >  
> > @@ -76,9 +76,10 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
> >  	size += addr - base;
> >  
> >  	if ((top_safe - base) < size) {
> > -		printf("phys_alloc: requested=0x%llx (align=0x%llx), "
> > -		       "need=0x%llx, but free=0x%llx. "
> > -		       "top=0x%llx, top_safe=0x%llx\n",
> > +		printf("phys_alloc: requested=0x%" PRIx64
> > +		       " (align=0x%" PRIx64 "), "
> > +		       "need=0x%" PRIx64 ", but free=0x%" PRIx64 ". "
> > +		       "top=0x%" PRIx64 ", top_safe=0x%" PRIx64 "\n",
> >  		       size_orig, align, size, top_safe - base,
> >  		       top, top_safe);
> >  		spin_unlock(&lock);
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index ce09d34e658b3..36e300bf4d0ff 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -48,6 +48,18 @@ typedef _Bool		bool;
> >  #define false 0
> >  #define true  1
> >  
> > +#if __SIZEOF_LONG__ == 8
> > +#  define __PRI64_PREFIX	"l"
> > +#  define __PRIPTR_PREFIX	"l"
> > +#else
> > +#  define __PRI64_PREFIX	"ll"
> > +#  define __PRIPTR_PREFIX
> > +#endif
> > +#define PRId64  __PRI64_PREFIX	"d"
> > +#define PRIu64  __PRI64_PREFIX	"u"
> > +#define PRIx64  __PRI64_PREFIX	"x"
> > +#define PRIxPTR __PRIPTR_PREFIX	"x"
> 
> So libcflat uses the header stdint.h from the standard gcc installation
> to get the uint64_t and friends, but you define the PRIx64 etc. here
> manually? That sounds strange... You could simply include "inttypes.h"
> instead of "stdint.h" in this libcflat.h header file, then you would get
> PRIx64 and friends automatically instead.

We can only include headers that are provided by libgcc in arch-neutral
files like libcflat, in order for cross-compiling to work. inttypes.h
isn't available.

> 
> OTOH, for a stand-alone binary like kvm-unit-tests, it's also somewhat
> risky to compile without "-nostdinc" since unwanted headers might be
> included that way (e.g. what happens if someone writes "#include
> <stdio.h>" by accident? You likely get some nice confusion...). So maybe
> it would be better to compile with "-nostdinc" instead and define all
> necessary things manually here?

We don't have -nostdinc because sometimes the x86 tests cheat and use
standard includes. arm and powerpc Makefiles could/should add it though,
as they don't currently cheat, and it's probably best if they never do.

Thanks,
drew

> 
>  Thomas
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 3/4] arm64: fix printf format warnings

[Andrew Jones]

On Tue, Mar 01, 2016 at 06:01:33AM +0100, Thomas Huth wrote:
> On 29.02.2016 21:19, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/arm64/processor.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
> > index c553390c969ea..deeab4ec9c8ac 100644
> > --- a/lib/arm64/processor.c
> > +++ b/lib/arm64/processor.c
> > @@ -66,12 +66,12 @@ void show_regs(struct pt_regs *regs)
> >  {
> >  	int i;
> >  
> > -	printf("pc : [<%016llx>] lr : [<%016llx>] pstate: %08llx\n",
> > +	printf("pc : [<%016lx>] lr : [<%016lx>] pstate: %08lx\n",
> >  			regs->pc, regs->regs[30], regs->pstate);
> > -	printf("sp : %016llx\n", regs->sp);
> > +	printf("sp : %016lx\n", regs->sp);
> >  
> >  	for (i = 29; i >= 0; --i) {
> > -		printf("x%-2d: %016llx ", i, regs->regs[i]);
> > +		printf("x%-2d: %016lx ", i, regs->regs[i]);
> >  		if (i % 2 == 0)
> >  			printf("\n");
> >  	}
> 
> Why don't you use PRIx64 here? The regs are defined as "u64", not as
> "long", so PRIx64 should fit better, shouldn't it?

This file is 64-bit only, never compiled for 32-bit, so long is
sufficient, and generates less clutter.

Thanks,
drew

Re: [kvm-unit-tests PATCH 4/4] x86: fix printf format warnings

[Andrew Jones]

On Tue, Mar 01, 2016 at 06:30:30AM +0100, Thomas Huth wrote:
> On 29.02.2016 21:19, Andrew Jones wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/x86/desc.c            |  6 +++---
> >  lib/x86/vm.c              |  6 +++---
> >  x86/access.c              |  4 ++--
> >  x86/eventinj.c            |  8 ++++----
> >  x86/kvmclock.c            |  2 +-
> >  x86/kvmclock_test.c       |  6 +++---
> >  x86/msr.c                 |  2 +-
> >  x86/s3.c                  |  6 +++---
> >  x86/svm.c                 | 12 ++++++------
> >  x86/taskswitch.c          |  2 +-
> >  x86/taskswitch2.c         |  2 +-
> >  x86/tsc.c                 |  4 ++--
> >  x86/tscdeadline_latency.c |  2 +-
> >  x86/vmx.c                 | 18 +++++++++---------
> >  x86/vmx_tests.c           | 20 ++++++++++----------
> >  x86/xsave.c               |  2 +-
> >  16 files changed, 51 insertions(+), 51 deletions(-)
> > 
> > diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> > index 1a80c01283853..0a79a41b56cb1 100644
> > --- a/lib/x86/desc.c
> > +++ b/lib/x86/desc.c
> > @@ -53,7 +53,7 @@ static void check_exception_table(struct ex_regs *regs)
> >              return;
> >          }
> >      }
> > -    printf("unhandled exception %d\n", regs->vector);
> > +    printf("unhandled exception %ld\n", regs->vector);
> 
> That could also be %lu instead, I think, but it likely does not really
> matter here.
> 
> >      exit(7);
> >  }
> >  
> > @@ -75,9 +75,9 @@ void do_handle_exception(struct ex_regs *regs)
> >  		exception_handlers[regs->vector](regs);
> >  		return;
> >  	}
> > -	printf("unhandled cpu exception %d\n", regs->vector);
> > +	printf("unhandled cpu exception %ld\n", regs->vector);
> >  	if (regs->vector == 14)
> > -		printf("PF at %p addr %p\n", regs->rip, read_cr2());
> > +		printf("PF at 0x%lx addr 0x%lx\n", regs->rip, read_cr2());
> >  	exit(7);
> >  }
> >  
> ...
> > diff --git a/x86/kvmclock_test.c b/x86/kvmclock_test.c
> > index b6da5ebbe1097..c8ffa126803d5 100644
> > --- a/x86/kvmclock_test.c
> > +++ b/x86/kvmclock_test.c
> > @@ -67,7 +67,7 @@ static void kvm_clock_test(void *data)
> >                          ++hv_test_info->warps;
> >                          if (delta < hv_test_info->worst){
> >                                  hv_test_info->worst = delta;
> > -                                printf("Worst warp %lld %\n", hv_test_info->worst);
> > +                                printf("Worst warp %lld\n", hv_test_info->worst);
> 
> Ah, nice, so this addition of __attribute__((format)) indeed revealed
> also some real problematic format strings :-)
> 
> >                          }
> >                          spin_unlock(&hv_test_info->lock);
> >                  }
> > @@ -102,8 +102,8 @@ static int cycle_test(int ncpus, int check, struct test_info *ti)
> >          printf("Total vcpus: %d\n", ncpus);
> >          printf("Test  loops: %ld\n", loops);
> >          if (check == 1) {
> > -                printf("Total warps:  %lld\n", ti->warps);
> > -                printf("Total stalls: %lld\n", ti->stalls);
> > +                printf("Total warps:  %" PRId64 "\n", ti->warps);
> > +                printf("Total stalls: %" PRId64 "\n", ti->stalls);
> >                  printf("Worst warp:   %lld\n", ti->worst);
> >          } else
> >                  printf("TSC cycles:  %lld\n", end - begin);
> > diff --git a/x86/msr.c b/x86/msr.c
> > index ec4710ed478b9..50b13840ed946 100644
> > --- a/x86/msr.c
> > +++ b/x86/msr.c
> > @@ -88,7 +88,7 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l
> >      wrmsr(msr_index, input);
> >      r = rdmsr(msr_index);
> >      if (expected != r) {
> > -        printf("testing %s: output = 0x%x:0x%x expected = 0x%x:0x%x\n", sptr, r >> 32, r, expected >> 32, expected);
> > +        printf("testing %s: output = 0x%llx:0x%llx expected = 0x%llx:0x%llx\n", sptr, r >> 32, r, expected >> 32, expected);
> 
> That code looks wrong (before and after your change, but in different
> ways ;-)):
> r is long long, so let's assume its value is 0x1234567890ABCDEF. You
> would then get the following output:
> 
>  output = 0x12345678:0x1234567890ABCDEF
> 
> But the author likely intended:
> 
>  output = 0x12345678:0x90ABCDEF
> 
> Same for the "expected" value.
> 
> So either the lower part has explicitely be casted to (uint32_t), or
> simply print the whole value only once with llx, without the colon
> inbetween.

Uff, I guess I was in monkey mode at this point. I'll change it to the
last suggestion, no ':'.

> 
> >      }
> >      report(sptr, expected == r);
> >  }
> ...
> 
> > diff --git a/x86/svm.c b/x86/svm.c
> > index 1046ddf73732f..934b2ae91fa81 100644
> > --- a/x86/svm.c
> > +++ b/x86/svm.c
> > @@ -934,9 +934,9 @@ static bool latency_finished(struct test *test)
> >  
> >  static bool latency_check(struct test *test)
> >  {
> > -    printf("    Latency VMRUN : max: %d min: %d avg: %d\n", latvmrun_max,
> > +    printf("    Latency VMRUN : max: %ld min: %ld avg: %ld\n", latvmrun_max,
> >              latvmrun_min, vmrun_sum / LATENCY_RUNS);
> > -    printf("    Latency VMEXIT: max: %d min: %d avg: %d\n", latvmexit_max,
> > +    printf("    Latency VMEXIT: max: %ld min: %ld avg: %ld\n", latvmexit_max,
> >              latvmexit_min, vmexit_sum / LATENCY_RUNS);
> >      return true;
> >  }
> > @@ -998,13 +998,13 @@ static bool lat_svm_insn_finished(struct test *test)
> >  
> >  static bool lat_svm_insn_check(struct test *test)
> >  {
> > -    printf("    Latency VMLOAD: max: %d min: %d avg: %d\n", latvmload_max,
> > +    printf("    Latency VMLOAD: max: %ld min: %ld avg: %ld\n", latvmload_max,
> >              latvmload_min, vmload_sum / LATENCY_RUNS);
> > -    printf("    Latency VMSAVE: max: %d min: %d avg: %d\n", latvmsave_max,
> > +    printf("    Latency VMSAVE: max: %ld min: %ld avg: %ld\n", latvmsave_max,
> >              latvmsave_min, vmsave_sum / LATENCY_RUNS);
> > -    printf("    Latency STGI:   max: %d min: %d avg: %d\n", latstgi_max,
> > +    printf("    Latency STGI:   max: %ld min: %ld avg: %ld\n", latstgi_max,
> >              latstgi_min, stgi_sum / LATENCY_RUNS);
> > -    printf("    Latency CLGI:   max: %d min: %d avg: %d\n", latclgi_max,
> > +    printf("    Latency CLGI:   max: %ld min: %ld avg: %ld\n", latclgi_max,
> >              latclgi_min, clgi_sum / LATENCY_RUNS);
> >      return true;
> >  }
> 
> The latxxx_min/max variables seem to be declared with u64 ... so maybe
> better use PRIu64 here instead?

svm unit tests are 64-bit only, so this should be OK.

> 
> > diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
> > index db3e41aa82186..dbc9a2525d01a 100644
> > --- a/x86/taskswitch2.c
> > +++ b/x86/taskswitch2.c
> > @@ -62,7 +62,7 @@ start:
> >  
> >  void do_pf_tss(ulong *error_code)
> >  {
> > -	printf("PF task is running %x %x\n", error_code, *(ulong*)error_code);
> > +	printf("PF task is running %p %lx\n", error_code, *(ulong*)error_code);
> 
> While you're at it, you could remove the (ulong*) typecast here, too.

Oh yeah, that's indeed pointless.

> 
> >  	print_current_tss_info();
> >  	if (*(ulong*)error_code == 0x2) /* write access, not present */
> >  		test_count++;
> ...
> > diff --git a/x86/vmx.c b/x86/vmx.c
> > index 3fa1a735881f7..e047a59ab1129 100644
> > --- a/x86/vmx.c
> > +++ b/x86/vmx.c
> > @@ -144,17 +144,17 @@ void print_vmexit_info()
> >  	guest_rip = vmcs_read(GUEST_RIP);
> >  	guest_rsp = vmcs_read(GUEST_RSP);
> >  	printf("VMEXIT info:\n");
> > -	printf("\tvmexit reason = %d\n", reason);
> > -	printf("\texit qualification = 0x%x\n", exit_qual);
> > -	printf("\tBit 31 of reason = %x\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> > -	printf("\tguest_rip = 0x%llx\n", guest_rip);
> > -	printf("\tRAX=0x%llx    RBX=0x%llx    RCX=0x%llx    RDX=0x%llx\n",
> > +	printf("\tvmexit reason = %ld\n", reason);
> > +	printf("\texit qualification = 0x%lx\n", exit_qual);
> > +	printf("\tBit 31 of reason = %lx\n", (vmcs_read(EXI_REASON) >> 31) & 1);
> > +	printf("\tguest_rip = 0x%lx\n", guest_rip);
> > +	printf("\tRAX=0x%lx    RBX=0x%lx    RCX=0x%lx    RDX=0x%lx\n",
> >  		regs.rax, regs.rbx, regs.rcx, regs.rdx);
> > -	printf("\tRSP=0x%llx    RBP=0x%llx    RSI=0x%llx    RDI=0x%llx\n",
> > +	printf("\tRSP=0x%lx    RBP=0x%lx    RSI=0x%lx    RDI=0x%lx\n",
> >  		guest_rsp, regs.rbp, regs.rsi, regs.rdi);
> > -	printf("\tR8 =0x%llx    R9 =0x%llx    R10=0x%llx    R11=0x%llx\n",
> > +	printf("\tR8 =0x%lx    R9 =0x%lx    R10=0x%lx    R11=0x%lx\n",
> >  		regs.r8, regs.r9, regs.r10, regs.r11);
> > -	printf("\tR12=0x%llx    R13=0x%llx    R14=0x%llx    R15=0x%llx\n",
> > +	printf("\tR12=0x%lx    R13=0x%lx    R14=0x%lx    R15=0x%lx\n",
> >  		regs.r12, regs.r13, regs.r14, regs.r15);
> >  }
> 
> The registers are declared as u64 ... so use PRIx64 for them instead?

64-bit only file.

> 
> > @@ -842,7 +842,7 @@ static int handle_hypercall()
> >  	case HYPERCALL_VMEXIT:
> >  		return VMX_TEST_VMEXIT;
> >  	default:
> > -		printf("ERROR : Invalid hypercall number : %d\n", hypercall_no);
> > +		printf("ERROR : Invalid hypercall number : %ld\n", hypercall_no);
> >  	}
> >  	return VMX_TEST_EXIT;
> >  }
> ...
> > diff --git a/x86/xsave.c b/x86/xsave.c
> > index e471835b42fd9..52142d2cdb93b 100644
> > --- a/x86/xsave.c
> > +++ b/x86/xsave.c
> > @@ -75,7 +75,7 @@ void test_xsave(void)
> >      printf("Legal instruction testing:\n");
> >  
> >      supported_xcr0 = get_supported_xcr0();
> > -    printf("Supported XCR0 bits: 0x%x\n", supported_xcr0);
> > +    printf("Supported XCR0 bits: 0x%lx\n", supported_xcr0);
> 
> Also PRIx64 here?

Also 64-bit only

Thanks,
drew

Re: [kvm-unit-tests PATCH 1/4] lib: *printf: warn on format/arg mismatch

[Paolo Bonzini]

On 29/02/2016 21:19, Andrew Jones wrote:
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/libcflat.h | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index 39100f29713b8..ce09d34e658b3 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -51,13 +51,15 @@ typedef _Bool		bool;
>  extern void puts(const char *s);
>  extern void exit(int code);
>  extern void abort(void);
> -
> -extern int printf(const char *fmt, ...);
> -extern int snprintf(char *buf, int size, const char *fmt, ...);
> -extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
> -extern int vprintf(const char *fmt, va_list va);
>  extern long atol(const char *ptr);
>  
> +#define _f(i,j) __attribute__((format(printf, i, j)))

Please name this ATTRIBUTE_PRINTF.

Paolo

> +extern int printf(const char *fmt, ...) _f(1,2);
> +extern int snprintf(char *buf, int size, const char *fmt, ...) _f(3,4);
> +extern int vsnprintf(char *buf, int size, const char *fmt, va_list va) _f(3,0);
> +extern int vprintf(const char *fmt, va_list va) _f(1,0);
> +#undef _f
> +
>  extern void report_prefix_push(const char *prefix);
>  extern void report_prefix_pop(void);
>  extern void report(const char *msg_fmt, bool pass, ...);
> 

Re: [kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Paolo Bonzini]

On 29/02/2016 21:19, Andrew Jones wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  lib/alloc.c    | 13 +++++++------
>  lib/libcflat.h | 12 ++++++++++++
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/alloc.c b/lib/alloc.c
> index 34f71a337d868..e345843b7fe53 100644
> --- a/lib/alloc.c
> +++ b/lib/alloc.c
> @@ -28,13 +28,13 @@ void phys_alloc_show(void)
>  	int i;
>  
>  	spin_lock(&lock);
> -	printf("phys_alloc minimum alignment: 0x%llx\n", align_min);
> +	printf("phys_alloc minimum alignment: 0x%" PRIx64 "\n", align_min);
>  	for (i = 0; i < nr_regions; ++i)
> -		printf("%016llx-%016llx [%s]\n",
> +		printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n",
>  			regions[i].base,
>  			regions[i].base + regions[i].size - 1,
>  			"USED");
> -	printf("%016llx-%016llx [%s]\n", base, top - 1, "FREE");
> +	printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", base, top - 1, "FREE");
>  	spin_unlock(&lock);
>  }
>  
> @@ -76,9 +76,10 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size,
>  	size += addr - base;
>  
>  	if ((top_safe - base) < size) {
> -		printf("phys_alloc: requested=0x%llx (align=0x%llx), "
> -		       "need=0x%llx, but free=0x%llx. "
> -		       "top=0x%llx, top_safe=0x%llx\n",
> +		printf("phys_alloc: requested=0x%" PRIx64
> +		       " (align=0x%" PRIx64 "), "
> +		       "need=0x%" PRIx64 ", but free=0x%" PRIx64 ". "
> +		       "top=0x%" PRIx64 ", top_safe=0x%" PRIx64 "\n",
>  		       size_orig, align, size, top_safe - base,
>  		       top, top_safe);
>  		spin_unlock(&lock);
> diff --git a/lib/libcflat.h b/lib/libcflat.h
> index ce09d34e658b3..36e300bf4d0ff 100644
> --- a/lib/libcflat.h
> +++ b/lib/libcflat.h
> @@ -48,6 +48,18 @@ typedef _Bool		bool;
>  #define false 0
>  #define true  1
>  
> +#if __SIZEOF_LONG__ == 8
> +#  define __PRI64_PREFIX	"l"
> +#  define __PRIPTR_PREFIX	"l"
> +#else
> +#  define __PRI64_PREFIX	"ll"
> +#  define __PRIPTR_PREFIX
> +#endif
> +#define PRId64  __PRI64_PREFIX	"d"
> +#define PRIu64  __PRI64_PREFIX	"u"
> +#define PRIx64  __PRI64_PREFIX	"x"
> +#define PRIxPTR __PRIPTR_PREFIX	"x"

It's sad that (because of GCC's stdint.h) we cannot just define the
64-bit prefix to "ll" and the pointer-sized prefix to "l"...

Paolo

>  extern void puts(const char *s);
>  extern void exit(int code);
>  extern void abort(void);
> 

Re: [kvm-unit-tests PATCH 4/4] x86: fix printf format warnings

[Paolo Bonzini]

On 01/03/2016 10:24, Andrew Jones wrote:
>> > So either the lower part has explicitely be casted to (uint32_t), or
>> > simply print the whole value only once with llx, without the colon
>> > inbetween.
> Uff, I guess I was in monkey mode at this point. I'll change it to the
> last suggestion, no ':'.

There are cases where the result is split in two 32-bit values (e.g.
IA32_VMX_PROCBASED_CTLS) so the ":" is nice to have.

Paolo

Re: [kvm-unit-tests PATCH 1/4] lib: *printf: warn on format/arg mismatch

[Andrew Jones]

On Tue, Mar 01, 2016 at 11:43:06AM +0100, Paolo Bonzini wrote:
> 
> 
> On 29/02/2016 21:19, Andrew Jones wrote:
> > Suggested-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/libcflat.h | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/libcflat.h b/lib/libcflat.h
> > index 39100f29713b8..ce09d34e658b3 100644
> > --- a/lib/libcflat.h
> > +++ b/lib/libcflat.h
> > @@ -51,13 +51,15 @@ typedef _Bool		bool;
> >  extern void puts(const char *s);
> >  extern void exit(int code);
> >  extern void abort(void);
> > -
> > -extern int printf(const char *fmt, ...);
> > -extern int snprintf(char *buf, int size, const char *fmt, ...);
> > -extern int vsnprintf(char *buf, int size, const char *fmt, va_list va);
> > -extern int vprintf(const char *fmt, va_list va);
> >  extern long atol(const char *ptr);
> >  
> > +#define _f(i,j) __attribute__((format(printf, i, j)))
> 
> Please name this ATTRIBUTE_PRINTF.

Do you want me to keep it defined? I used _f, only because it's short,
keeping it from bloating the below lines, and then I undef'ed it when
it no longer had any purpose. If we'd rather not using something like
_f for the name, then I think we might as well just use the
__attribute__ string as is.

Thanks,
drew

> 
> Paolo
> 
> > +extern int printf(const char *fmt, ...) _f(1,2);
> > +extern int snprintf(char *buf, int size, const char *fmt, ...) _f(3,4);
> > +extern int vsnprintf(char *buf, int size, const char *fmt, va_list va) _f(3,0);
> > +extern int vprintf(const char *fmt, va_list va) _f(1,0);
> > +#undef _f
> > +
> >  extern void report_prefix_push(const char *prefix);
> >  extern void report_prefix_pop(void);
> >  extern void report(const char *msg_fmt, bool pass, ...);
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 4/4] x86: fix printf format warnings

[Andrew Jones]

On Tue, Mar 01, 2016 at 11:52:04AM +0100, Paolo Bonzini wrote:
> 
> 
> On 01/03/2016 10:24, Andrew Jones wrote:
> >> > So either the lower part has explicitely be casted to (uint32_t), or
> >> > simply print the whole value only once with llx, without the colon
> >> > inbetween.
> > Uff, I guess I was in monkey mode at this point. I'll change it to the
> > last suggestion, no ':'.
> 
> There are cases where the result is split in two 32-bit values (e.g.
> IA32_VMX_PROCBASED_CTLS) so the ":" is nice to have.

OK, I'll keep the ':'

Thanks,
drew

Re: [kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Paolo Bonzini]

On 01/03/2016 10:15, Andrew Jones wrote:
>> > So libcflat uses the header stdint.h from the standard gcc installation
>> > to get the uint64_t and friends, but you define the PRIx64 etc. here
>> > manually? That sounds strange... You could simply include "inttypes.h"
>> > instead of "stdint.h" in this libcflat.h header file, then you would get
>> > PRIx64 and friends automatically instead.
> We can only include headers that are provided by libgcc in arch-neutral
> files like libcflat, in order for cross-compiling to work. inttypes.h
> isn't available.

Yup, inttypes.h is not part of the freestanding environment.

> > OTOH, for a stand-alone binary like kvm-unit-tests, it's also somewhat
> > risky to compile without "-nostdinc" since unwanted headers might be
> > included that way (e.g. what happens if someone writes "#include
> > <stdio.h>" by accident? You likely get some nice confusion...). So maybe
> > it would be better to compile with "-nostdinc" instead and define all
> > necessary things manually here?
>
> We don't have -nostdinc because sometimes the x86 tests cheat and use
> standard includes. arm and powerpc Makefiles could/should add it though,
> as they don't currently cheat, and it's probably best if they never do.

Some of the includes are part of the freestanding environment: float.h,
iso646.h, limits.h, stdarg.h, stdbool.h, stddef.h, stdint.h and probably
a few more in C11 (e.g. stdalign.h, stdatomic.h, stdnoreturn.h).  Using
them is not cheating.

Paolo

Re: [kvm-unit-tests PATCH 2/4] lib/alloc: fix format warnings, add/use PRIx64

[Andrew Jones]

On Tue, Mar 01, 2016 at 12:04:01PM +0100, Paolo Bonzini wrote:
> 
> 
> On 01/03/2016 10:15, Andrew Jones wrote:
> >> > So libcflat uses the header stdint.h from the standard gcc installation
> >> > to get the uint64_t and friends, but you define the PRIx64 etc. here
> >> > manually? That sounds strange... You could simply include "inttypes.h"
> >> > instead of "stdint.h" in this libcflat.h header file, then you would get
> >> > PRIx64 and friends automatically instead.
> > We can only include headers that are provided by libgcc in arch-neutral
> > files like libcflat, in order for cross-compiling to work. inttypes.h
> > isn't available.
> 
> Yup, inttypes.h is not part of the freestanding environment.
> 
> > > OTOH, for a stand-alone binary like kvm-unit-tests, it's also somewhat
> > > risky to compile without "-nostdinc" since unwanted headers might be
> > > included that way (e.g. what happens if someone writes "#include
> > > <stdio.h>" by accident? You likely get some nice confusion...). So maybe
> > > it would be better to compile with "-nostdinc" instead and define all
> > > necessary things manually here?
> >
> > We don't have -nostdinc because sometimes the x86 tests cheat and use
> > standard includes. arm and powerpc Makefiles could/should add it though,
> > as they don't currently cheat, and it's probably best if they never do.
> 
> Some of the includes are part of the freestanding environment: float.h,
> iso646.h, limits.h, stdarg.h, stdbool.h, stddef.h, stdint.h and probably
> a few more in C11 (e.g. stdalign.h, stdatomic.h, stdnoreturn.h).  Using
> them is not cheating.

Ah, OK.

Actually, I just did a quick experiment. I added '-isysroot /foo' to
CFLAGS, in order to allow libgcc headers, but not anything in /usr/include.
Almost everything compiled, but

Bug report: x86/setjmp.c currently includes stdio.h

Thanks,
drew