[PATCH] mini-os: enable compiler check for printk format types

[PATCH] mini-os: enable compiler check for printk format types

[Thomas Leonard]

Signed-off-by: Thomas Leonard <talex5@gmail.com>
---

I tried compiling mini-os and stubdom on x86 and x86_64 and fixed all the
warnings reported in the most obvious way I could see.

 extras/mini-os/arch/x86/mm.c     | 22 +++++++++++-----------
 extras/mini-os/arch/x86/sched.c  |  4 ++--
 extras/mini-os/arch/x86/traps.c  | 18 +++++++++---------
 extras/mini-os/blkfront.c        |  2 +-
 extras/mini-os/include/console.h |  4 ++--
 extras/mini-os/lib/sys.c         |  4 ++--
 extras/mini-os/lwip-arch.c       |  2 +-
 extras/mini-os/minios.mk         |  2 +-
 extras/mini-os/mm.c              |  6 +++++-
 extras/mini-os/netfront.c        |  4 ++--
 extras/mini-os/sched.c           |  2 +-
 extras/mini-os/test.c            |  6 ++++--
 extras/mini-os/tpm_tis.c         |  8 ++++----
 extras/mini-os/xenbus/xenbus.c   |  4 ++--
 stubdom/vtpmmgr/disk_read.c      |  2 +-
 15 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/extras/mini-os/arch/x86/mm.c b/extras/mini-os/arch/x86/mm.c
index 9c6d1b8..186659d 100644
--- a/extras/mini-os/arch/x86/mm.c
+++ b/extras/mini-os/arch/x86/mm.c
@@ -94,7 +94,7 @@ static void new_pt_frame(unsigned long *pt_pfn, unsigned long prev_l_mfn,
         break;
 #endif
     default:
-        printk("new_pt_frame() called with invalid level number %d\n", level);
+        printk("new_pt_frame() called with invalid level number %lu\n", level);
         do_exit();
         break;
     }
@@ -181,7 +181,7 @@ static int need_pt_frame(unsigned long va, int level)
         if ( level == L1_FRAME )
             return 1;
 
-    printk("ERROR: Unknown frame level %d, hypervisor %llx,%llx\n", 
+    printk("ERROR: Unknown frame level %d, hypervisor %lx,%lx\n", 
            level, hyp_virt_start, hyp_virt_end);
     return -1;
 }
@@ -206,11 +206,11 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
     if ( *max_pfn >= virt_to_pfn(HYPERVISOR_VIRT_START) )
     {
         printk("WARNING: Mini-OS trying to use Xen virtual space. "
-               "Truncating memory from %dMB to ",
+               "Truncating memory from %luMB to ",
                ((unsigned long)pfn_to_virt(*max_pfn) -
                 (unsigned long)&_text)>>20);
         *max_pfn = virt_to_pfn(HYPERVISOR_VIRT_START - PAGE_SIZE);
-        printk("%dMB\n",
+        printk("%luMB\n",
                ((unsigned long)pfn_to_virt(*max_pfn) - 
                 (unsigned long)&_text)>>20);
     }
@@ -326,7 +326,7 @@ static void set_readonly(void *text, void *etext)
             count++;
         }
         else
-            printk("skipped %p\n", start_address);
+            printk("skipped %lx\n", start_address);
 
         start_address += PAGE_SIZE;
 
@@ -369,21 +369,21 @@ int mem_test(unsigned long *start_va, unsigned long *end_va, int verbose)
     /* write values and print page walks */
     if ( verbose && (((unsigned long)start_va) & 0xfffff) )
     {
-        printk("MemTest Start: 0x%lx\n", start_va);
+        printk("MemTest Start: 0x%p\n", start_va);
         page_walk((unsigned long)start_va);
     }
     for ( pointer = start_va; pointer < end_va; pointer++ )
     {
         if ( verbose && !(((unsigned long)pointer) & 0xfffff) )
         {
-            printk("Writing to %lx\n", pointer);
+            printk("Writing to %p\n", pointer);
             page_walk((unsigned long)pointer);
         }
         *pointer = (unsigned long)pointer & ~mask;
     }
     if ( verbose && (((unsigned long)end_va) & 0xfffff) )
     {
-        printk("MemTest End: %lx\n", end_va-1);
+        printk("MemTest End: %p\n", end_va-1);
         page_walk((unsigned long)end_va-1);
     }
  
@@ -516,7 +516,7 @@ void arch_init_demand_mapping_area(unsigned long cur_pfn)
 
     demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
     cur_pfn += DEMAND_MAP_PAGES;
-    printk("Demand map pfns at %lx-%lx.\n", 
+    printk("Demand map pfns at %lx-%p.\n", 
            demand_map_area_start, pfn_to_virt(cur_pfn));
 
 #ifdef HAVE_LIBC
@@ -619,7 +619,7 @@ void do_map_frames(unsigned long va,
                 if (err)
                     err[done * stride] = rc;
                 else {
-                    printk("Map %ld (%lx, ...) at %p failed: %d.\n",
+                    printk("Map %ld (%lx, ...) at %lx failed: %d.\n",
                            todo, mfns[done * stride] + done * incr, va, rc);
                     do_exit();
                 }
@@ -793,7 +793,7 @@ unsigned long alloc_contig_pages(int order, unsigned int addr_bits)
     out_frames = virt_to_pfn(in_va); /* PFNs to populate */
     ret = HYPERVISOR_memory_op(XENMEM_exchange, &exchange);
     if ( ret ) {
-        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%d\n", 
+        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%lu\n",
                order, ret, exchange.nr_exchanged);
         /* we still need to return the allocated pages above to the pool
          * ie. map them back into the 1:1 mapping etc. so we continue but 
diff --git a/extras/mini-os/arch/x86/sched.c b/extras/mini-os/arch/x86/sched.c
index e4a3dc2..ec13694 100644
--- a/extras/mini-os/arch/x86/sched.c
+++ b/extras/mini-os/arch/x86/sched.c
@@ -73,7 +73,7 @@ void dump_stack(struct thread *thread)
     printk("The stack for \"%s\"\n", thread->name);
     for(count = 0; count < 25 && pointer < bottom; count ++)
     {
-        printk("[0x%lx] 0x%lx\n", pointer, *pointer);
+        printk("[0x%p] 0x%lx\n", pointer, *pointer);
         pointer++;
     }
     
@@ -101,7 +101,7 @@ struct thread* arch_create_thread(char *name, void (*function)(void *),
     /* We can't use lazy allocation here since the trap handler runs on the stack */
     thread->stack = (char *)alloc_pages(STACK_SIZE_PAGE_ORDER);
     thread->name = name;
-    printk("Thread \"%s\": pointer: 0x%lx, stack: 0x%lx\n", name, thread, 
+    printk("Thread \"%s\": pointer: 0x%p, stack: 0x%p\n", name, thread, 
             thread->stack);
     
     thread->sp = (unsigned long)thread->stack + STACK_SIZE;
diff --git a/extras/mini-os/arch/x86/traps.c b/extras/mini-os/arch/x86/traps.c
index 516d133..6353718 100644
--- a/extras/mini-os/arch/x86/traps.c
+++ b/extras/mini-os/arch/x86/traps.c
@@ -34,14 +34,14 @@ void dump_regs(struct pt_regs *regs)
 {
     printk("Thread: %s\n", current->name);
 #ifdef __i386__    
-    printk("EIP: %x, EFLAGS %x.\n", regs->eip, regs->eflags);
-    printk("EBX: %08x ECX: %08x EDX: %08x\n",
+    printk("EIP: %lx, EFLAGS %lx.\n", regs->eip, regs->eflags);
+    printk("EBX: %08lx ECX: %08lx EDX: %08lx\n",
 	   regs->ebx, regs->ecx, regs->edx);
-    printk("ESI: %08x EDI: %08x EBP: %08x EAX: %08x\n",
+    printk("ESI: %08lx EDI: %08lx EBP: %08lx EAX: %08lx\n",
 	   regs->esi, regs->edi, regs->ebp, regs->eax);
-    printk("DS: %04x ES: %04x orig_eax: %08x, eip: %08x\n",
+    printk("DS: %04x ES: %04x orig_eax: %08lx, eip: %08lx\n",
 	   regs->xds, regs->xes, regs->orig_eax, regs->eip);
-    printk("CS: %04x EFLAGS: %08x esp: %08x ss: %04x\n",
+    printk("CS: %04x EFLAGS: %08lx esp: %08lx ss: %04x\n",
 	   regs->xcs, regs->eflags, regs->esp, regs->xss);
 #else
     printk("RIP: %04lx:[<%016lx>] ", regs->cs & 0xffff, regs->rip);
@@ -214,10 +214,10 @@ void do_page_fault(struct pt_regs *regs, unsigned long error_code)
     barrier();
 
 #if defined(__x86_64__)
-    printk("Page fault at linear address %p, rip %p, regs %p, sp %p, our_sp %p, code %lx\n",
+    printk("Page fault at linear address %lx, rip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
            addr, regs->rip, regs, regs->rsp, &addr, error_code);
 #else
-    printk("Page fault at linear address %p, eip %p, regs %p, sp %p, our_sp %p, code %lx\n",
+    printk("Page fault at linear address %lx, eip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
            addr, regs->eip, regs, regs->esp, &addr, error_code);
 #endif
 
@@ -243,9 +243,9 @@ void do_general_protection(struct pt_regs *regs, long error_code)
 {
     struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_crash };
 #ifdef __i386__
-    printk("GPF eip: %p, error_code=%lx\n", regs->eip, error_code);
+    printk("GPF eip: %lx, error_code=%lx\n", regs->eip, error_code);
 #else    
-    printk("GPF rip: %p, error_code=%lx\n", regs->rip, error_code);
+    printk("GPF rip: %lx, error_code=%lx\n", regs->rip, error_code);
 #endif
     dump_regs(regs);
 #if defined(__x86_64__)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index 59e576f..bdb7765 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -236,7 +236,7 @@ done:
     }
     unmask_evtchn(dev->evtchn);
 
-    printk("%u sectors of %u bytes\n", dev->info.sectors, dev->info.sector_size);
+    printk("%lu sectors of %u bytes\n", (unsigned long) dev->info.sectors, dev->info.sector_size);
     printk("**************************\n");
 
     return dev;
diff --git a/extras/mini-os/include/console.h b/extras/mini-os/include/console.h
index 3755b66..a77f47f 100644
--- a/extras/mini-os/include/console.h
+++ b/extras/mini-os/include/console.h
@@ -64,8 +64,8 @@ struct consfront_dev {
 
 
 void print(int direct, const char *fmt, va_list args);
-void printk(const char *fmt, ...);
-void xprintk(const char *fmt, ...);
+void printk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
+void xprintk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
 
 #define tprintk(_fmt, _args...) printk("[%s] " _fmt, current->name, ##_args) 
 
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index 13e7e59..1e9bddc 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -1319,7 +1319,7 @@ int clock_gettime(clockid_t clk_id, struct timespec *tp)
 	    break;
 	}
 	default:
-	    print_unsupported("clock_gettime(%d)", clk_id);
+	    print_unsupported("clock_gettime(%ld)", (long) clk_id);
 	    errno = EINVAL;
 	    return -1;
     }
@@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
         mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
     }
 
-    printk("sparsing %ldMB at %lx\n", size >> 20, data);
+    printk("sparsing %ldMB at %lx\n", (long) size >> 20, data);
 
     munmap((void *) data, size);
     free_physical_pages(mfns, n);
diff --git a/extras/mini-os/lwip-arch.c b/extras/mini-os/lwip-arch.c
index e634ef4..e2476d4 100644
--- a/extras/mini-os/lwip-arch.c
+++ b/extras/mini-os/lwip-arch.c
@@ -236,7 +236,7 @@ sys_thread_t sys_thread_new(char *name, void (* thread)(void *arg), void *arg, i
 {
     struct thread *t;
     if (stacksize > STACK_SIZE) {
-	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, STACK_SIZE);
+	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, (int) STACK_SIZE);
 	do_exit();
     }
     lwip_thread = t = create_thread(name, thread, arg);
diff --git a/extras/mini-os/minios.mk b/extras/mini-os/minios.mk
index f42f48b..20ba64b 100644
--- a/extras/mini-os/minios.mk
+++ b/extras/mini-os/minios.mk
@@ -6,7 +6,7 @@ debug = y
 
 # Define some default flags.
 # NB. '-Wcast-qual' is nasty, so I omitted it.
-DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls
+DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls -Wformat
 DEF_CFLAGS += $(call cc-option,$(CC),-fno-stack-protector,)
 DEF_CFLAGS += $(call cc-option,$(CC),-fgnu89-inline)
 DEF_CFLAGS += -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline
diff --git a/extras/mini-os/mm.c b/extras/mini-os/mm.c
index 64b3292..31aaf83 100644
--- a/extras/mini-os/mm.c
+++ b/extras/mini-os/mm.c
@@ -379,7 +379,11 @@ void *sbrk(ptrdiff_t increment)
     unsigned long new_brk = old_brk + increment;
 
     if (new_brk > heap_end) {
-	printk("Heap exhausted: %p + %lx = %p > %p\n", old_brk, increment, new_brk, heap_end);
+	printk("Heap exhausted: %lx + %lx = %p > %p\n",
+			old_brk,
+			(unsigned long) increment,
+			(void *) new_brk,
+			(void *) heap_end);
 	return NULL;
     }
     
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 44c3995..6f335fe 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
     dev->fd = -1;
 #endif
 
-    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
-    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
+    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
+    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
     init_SEMAPHORE(&dev->tx_sem, NET_TX_RING_SIZE);
     for(i=0;i<NET_TX_RING_SIZE;i++)
     {
diff --git a/extras/mini-os/sched.c b/extras/mini-os/sched.c
index d0c607e..1e843d9 100644
--- a/extras/mini-os/sched.c
+++ b/extras/mini-os/sched.c
@@ -276,7 +276,7 @@ void th_f2(void *data)
 {
     for(;;)
     {
-        printk("Thread OTHER executing, data 0x%lx\n", data);
+        printk("Thread OTHER executing, data 0x%p\n", data);
         schedule();
     }
 }
diff --git a/extras/mini-os/test.c b/extras/mini-os/test.c
index 20d372b..64a4057 100644
--- a/extras/mini-os/test.c
+++ b/extras/mini-os/test.c
@@ -116,7 +116,7 @@ static void blk_read_completed(struct blkfront_aiocb *aiocb, int ret)
 {
     struct blk_req *req = aiocb->data;
     if (ret)
-        printk("got error code %d when reading at offset %ld\n", ret, aiocb->aio_offset);
+        printk("got error code %d when reading at offset %ld\n", ret, (long) aiocb->aio_offset);
     else
         blk_size_read += blk_info.sector_size;
     free(aiocb->aio_buf);
@@ -237,7 +237,9 @@ static void blkfront_thread(void *p)
         blkfront_aio_poll(blk_dev);
         gettimeofday(&tv, NULL);
         if (tv.tv_sec > lasttime + 10) {
-            printk("%llu read, %llu write\n", blk_size_read, blk_size_write);
+            printk("%llu read, %llu write\n",
+                    (unsigned long long) blk_size_read,
+                    (unsigned long long) blk_size_write);
             lasttime = tv.tv_sec;
         }
 
diff --git a/extras/mini-os/tpm_tis.c b/extras/mini-os/tpm_tis.c
index dc4134a..936e854 100644
--- a/extras/mini-os/tpm_tis.c
+++ b/extras/mini-os/tpm_tis.c
@@ -906,7 +906,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
    //down(&chip->tpm_mutex);
 
    if ((rc = tpm_tis_send(chip, (uint8_t *) buf, count)) < 0) {
-      printk("tpm_transmit: tpm_send: error %ld\n", rc);
+      printk("tpm_transmit: tpm_send: error %ld\n", (long) rc);
       goto out;
    }
 
@@ -938,7 +938,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
 
 out_recv:
    if((rc = tpm_tis_recv(chip, (uint8_t *) buf, bufsiz)) < 0) {
-      printk("tpm_transmit: tpm_recv: error %d\n", rc);
+      printk("tpm_transmit: tpm_recv: error %d\n", (int) rc);
    }
 out:
    //up(&chip->tpm_mutex);
@@ -977,7 +977,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 
    if((rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
 	 "attempting to determine the timeouts")) != 0) {
-      printk("transmit failed %d\n", rc);
+      printk("transmit failed %d\n", (int) rc);
       goto duration;
    }
 
@@ -1132,7 +1132,7 @@ struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned i
       if(locality_enabled(tpm, i)) {
 	 /* Map the page in now */
 	 if((tpm->pages[i] = ioremap_nocache(addr, PAGE_SIZE)) == NULL) {
-	    printk("Unable to map iomem page a address %p\n", addr);
+	    printk("Unable to map iomem page a address %lx\n", addr);
 	    goto abort_egress;
 	 }
 
diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c
index 934f23b..4613ed6 100644
--- a/extras/mini-os/xenbus/xenbus.c
+++ b/extras/mini-os/xenbus/xenbus.c
@@ -337,8 +337,8 @@ void init_xenbus(void)
 		      xenbus_evtchn_handler,
               NULL);
     unmask_evtchn(start_info.store_evtchn);
-    printk("xenbus initialised on irq %d mfn %#lx\n",
-	   err, start_info.store_mfn);
+    printk("xenbus initialised on irq %d mfn %#llx\n",
+	   err, (unsigned long long) start_info.store_mfn);
 }
 
 void fini_xenbus(void)
diff --git a/stubdom/vtpmmgr/disk_read.c b/stubdom/vtpmmgr/disk_read.c
index 33aacdd..a4c63c8 100644
--- a/stubdom/vtpmmgr/disk_read.c
+++ b/stubdom/vtpmmgr/disk_read.c
@@ -538,7 +538,7 @@ int vtpm_load_disk(void)
 	printk(" root seal: %lu; sector of %d: %lu\n",
 		sizeof(struct disk_root_sealed_data), SEALS_PER_ROOT_SEAL_LIST, sizeof(struct disk_seal_list));
 	printk(" root: %lu v=%lu\n", sizeof(root1), sizeof(root1.v));
-	printk(" itree: %lu; sector of %d: %lu\n",
+	printk(" itree: %u; sector of %d: %lu\n",
 		4 + 32, NR_ENTRIES_PER_ITREE, sizeof(struct disk_itree_sector));
 	printk(" group: %lu v=%lu id=%lu md=%lu\n",
 		sizeof(struct disk_group_sector), sizeof(struct disk_group_sector_mac3_area),
-- 
2.0.3

Re: [PATCH] mini-os: enable compiler check for printk format types

[Andrew Cooper]

On 06/08/2014 10:44, Thomas Leonard wrote:
> Signed-off-by: Thomas Leonard <talex5@gmail.com>
> ---
>
> I tried compiling mini-os and stubdom on x86 and x86_64 and fixed all the
> warnings reported in the most obvious way I could see.
>
>  extras/mini-os/arch/x86/mm.c     | 22 +++++++++++-----------
>  extras/mini-os/arch/x86/sched.c  |  4 ++--
>  extras/mini-os/arch/x86/traps.c  | 18 +++++++++---------
>  extras/mini-os/blkfront.c        |  2 +-
>  extras/mini-os/include/console.h |  4 ++--
>  extras/mini-os/lib/sys.c         |  4 ++--
>  extras/mini-os/lwip-arch.c       |  2 +-
>  extras/mini-os/minios.mk         |  2 +-
>  extras/mini-os/mm.c              |  6 +++++-
>  extras/mini-os/netfront.c        |  4 ++--
>  extras/mini-os/sched.c           |  2 +-
>  extras/mini-os/test.c            |  6 ++++--
>  extras/mini-os/tpm_tis.c         |  8 ++++----
>  extras/mini-os/xenbus/xenbus.c   |  4 ++--
>  stubdom/vtpmmgr/disk_read.c      |  2 +-
>  15 files changed, 48 insertions(+), 42 deletions(-)
>
> diff --git a/extras/mini-os/arch/x86/mm.c b/extras/mini-os/arch/x86/mm.c
> index 9c6d1b8..186659d 100644
> --- a/extras/mini-os/arch/x86/mm.c
> +++ b/extras/mini-os/arch/x86/mm.c
> @@ -94,7 +94,7 @@ static void new_pt_frame(unsigned long *pt_pfn, unsigned long prev_l_mfn,
>          break;
>  #endif
>      default:
> -        printk("new_pt_frame() called with invalid level number %d\n", level);
> +        printk("new_pt_frame() called with invalid level number %lu\n", level);
>          do_exit();
>          break;
>      }
> @@ -181,7 +181,7 @@ static int need_pt_frame(unsigned long va, int level)
>          if ( level == L1_FRAME )
>              return 1;
>  
> -    printk("ERROR: Unknown frame level %d, hypervisor %llx,%llx\n", 
> +    printk("ERROR: Unknown frame level %d, hypervisor %lx,%lx\n", 
>             level, hyp_virt_start, hyp_virt_end);
>      return -1;
>  }
> @@ -206,11 +206,11 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
>      if ( *max_pfn >= virt_to_pfn(HYPERVISOR_VIRT_START) )
>      {
>          printk("WARNING: Mini-OS trying to use Xen virtual space. "
> -               "Truncating memory from %dMB to ",
> +               "Truncating memory from %luMB to ",
>                 ((unsigned long)pfn_to_virt(*max_pfn) -
>                  (unsigned long)&_text)>>20);
>          *max_pfn = virt_to_pfn(HYPERVISOR_VIRT_START - PAGE_SIZE);
> -        printk("%dMB\n",
> +        printk("%luMB\n",
>                 ((unsigned long)pfn_to_virt(*max_pfn) - 
>                  (unsigned long)&_text)>>20);
>      }
> @@ -326,7 +326,7 @@ static void set_readonly(void *text, void *etext)
>              count++;
>          }
>          else
> -            printk("skipped %p\n", start_address);
> +            printk("skipped %lx\n", start_address);
>  
>          start_address += PAGE_SIZE;
>  
> @@ -369,21 +369,21 @@ int mem_test(unsigned long *start_va, unsigned long *end_va, int verbose)
>      /* write values and print page walks */
>      if ( verbose && (((unsigned long)start_va) & 0xfffff) )
>      {
> -        printk("MemTest Start: 0x%lx\n", start_va);
> +        printk("MemTest Start: 0x%p\n", start_va);
>          page_walk((unsigned long)start_va);
>      }
>      for ( pointer = start_va; pointer < end_va; pointer++ )
>      {
>          if ( verbose && !(((unsigned long)pointer) & 0xfffff) )
>          {
> -            printk("Writing to %lx\n", pointer);
> +            printk("Writing to %p\n", pointer);
>              page_walk((unsigned long)pointer);
>          }
>          *pointer = (unsigned long)pointer & ~mask;
>      }
>      if ( verbose && (((unsigned long)end_va) & 0xfffff) )
>      {
> -        printk("MemTest End: %lx\n", end_va-1);
> +        printk("MemTest End: %p\n", end_va-1);
>          page_walk((unsigned long)end_va-1);
>      }
>   
> @@ -516,7 +516,7 @@ void arch_init_demand_mapping_area(unsigned long cur_pfn)
>  
>      demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
>      cur_pfn += DEMAND_MAP_PAGES;
> -    printk("Demand map pfns at %lx-%lx.\n", 
> +    printk("Demand map pfns at %lx-%p.\n", 
>             demand_map_area_start, pfn_to_virt(cur_pfn));
>  
>  #ifdef HAVE_LIBC
> @@ -619,7 +619,7 @@ void do_map_frames(unsigned long va,
>                  if (err)
>                      err[done * stride] = rc;
>                  else {
> -                    printk("Map %ld (%lx, ...) at %p failed: %d.\n",
> +                    printk("Map %ld (%lx, ...) at %lx failed: %d.\n",
>                             todo, mfns[done * stride] + done * incr, va, rc);
>                      do_exit();
>                  }
> @@ -793,7 +793,7 @@ unsigned long alloc_contig_pages(int order, unsigned int addr_bits)
>      out_frames = virt_to_pfn(in_va); /* PFNs to populate */
>      ret = HYPERVISOR_memory_op(XENMEM_exchange, &exchange);
>      if ( ret ) {
> -        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%d\n", 
> +        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%lu\n",
>                 order, ret, exchange.nr_exchanged);
>          /* we still need to return the allocated pages above to the pool
>           * ie. map them back into the 1:1 mapping etc. so we continue but 
> diff --git a/extras/mini-os/arch/x86/sched.c b/extras/mini-os/arch/x86/sched.c
> index e4a3dc2..ec13694 100644
> --- a/extras/mini-os/arch/x86/sched.c
> +++ b/extras/mini-os/arch/x86/sched.c
> @@ -73,7 +73,7 @@ void dump_stack(struct thread *thread)
>      printk("The stack for \"%s\"\n", thread->name);
>      for(count = 0; count < 25 && pointer < bottom; count ++)
>      {
> -        printk("[0x%lx] 0x%lx\n", pointer, *pointer);
> +        printk("[0x%p] 0x%lx\n", pointer, *pointer);
>          pointer++;
>      }
>      
> @@ -101,7 +101,7 @@ struct thread* arch_create_thread(char *name, void (*function)(void *),
>      /* We can't use lazy allocation here since the trap handler runs on the stack */
>      thread->stack = (char *)alloc_pages(STACK_SIZE_PAGE_ORDER);
>      thread->name = name;
> -    printk("Thread \"%s\": pointer: 0x%lx, stack: 0x%lx\n", name, thread, 
> +    printk("Thread \"%s\": pointer: 0x%p, stack: 0x%p\n", name, thread, 
>              thread->stack);
>      
>      thread->sp = (unsigned long)thread->stack + STACK_SIZE;
> diff --git a/extras/mini-os/arch/x86/traps.c b/extras/mini-os/arch/x86/traps.c
> index 516d133..6353718 100644
> --- a/extras/mini-os/arch/x86/traps.c
> +++ b/extras/mini-os/arch/x86/traps.c
> @@ -34,14 +34,14 @@ void dump_regs(struct pt_regs *regs)
>  {
>      printk("Thread: %s\n", current->name);
>  #ifdef __i386__    
> -    printk("EIP: %x, EFLAGS %x.\n", regs->eip, regs->eflags);
> -    printk("EBX: %08x ECX: %08x EDX: %08x\n",
> +    printk("EIP: %lx, EFLAGS %lx.\n", regs->eip, regs->eflags);
> +    printk("EBX: %08lx ECX: %08lx EDX: %08lx\n",
>  	   regs->ebx, regs->ecx, regs->edx);
> -    printk("ESI: %08x EDI: %08x EBP: %08x EAX: %08x\n",
> +    printk("ESI: %08lx EDI: %08lx EBP: %08lx EAX: %08lx\n",
>  	   regs->esi, regs->edi, regs->ebp, regs->eax);
> -    printk("DS: %04x ES: %04x orig_eax: %08x, eip: %08x\n",
> +    printk("DS: %04x ES: %04x orig_eax: %08lx, eip: %08lx\n",
>  	   regs->xds, regs->xes, regs->orig_eax, regs->eip);
> -    printk("CS: %04x EFLAGS: %08x esp: %08x ss: %04x\n",
> +    printk("CS: %04x EFLAGS: %08lx esp: %08lx ss: %04x\n",
>  	   regs->xcs, regs->eflags, regs->esp, regs->xss);
>  #else
>      printk("RIP: %04lx:[<%016lx>] ", regs->cs & 0xffff, regs->rip);
> @@ -214,10 +214,10 @@ void do_page_fault(struct pt_regs *regs, unsigned long error_code)
>      barrier();
>  
>  #if defined(__x86_64__)
> -    printk("Page fault at linear address %p, rip %p, regs %p, sp %p, our_sp %p, code %lx\n",
> +    printk("Page fault at linear address %lx, rip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
>             addr, regs->rip, regs, regs->rsp, &addr, error_code);
>  #else
> -    printk("Page fault at linear address %p, eip %p, regs %p, sp %p, our_sp %p, code %lx\n",
> +    printk("Page fault at linear address %lx, eip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
>             addr, regs->eip, regs, regs->esp, &addr, error_code);
>  #endif
>  
> @@ -243,9 +243,9 @@ void do_general_protection(struct pt_regs *regs, long error_code)
>  {
>      struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_crash };
>  #ifdef __i386__
> -    printk("GPF eip: %p, error_code=%lx\n", regs->eip, error_code);
> +    printk("GPF eip: %lx, error_code=%lx\n", regs->eip, error_code);
>  #else    
> -    printk("GPF rip: %p, error_code=%lx\n", regs->rip, error_code);
> +    printk("GPF rip: %lx, error_code=%lx\n", regs->rip, error_code);
>  #endif
>      dump_regs(regs);
>  #if defined(__x86_64__)
> diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
> index 59e576f..bdb7765 100644
> --- a/extras/mini-os/blkfront.c
> +++ b/extras/mini-os/blkfront.c
> @@ -236,7 +236,7 @@ done:
>      }
>      unmask_evtchn(dev->evtchn);
>  
> -    printk("%u sectors of %u bytes\n", dev->info.sectors, dev->info.sector_size);
> +    printk("%lu sectors of %u bytes\n", (unsigned long) dev->info.sectors, dev->info.sector_size);

Use PRIu64 and no cast, or you will trucate the sector count for 32bit
builds.  In general you should never need to cast (unless you have
something which isn't a long but changes size between builds)

>      printk("**************************\n");
>  
>      return dev;
> diff --git a/extras/mini-os/include/console.h b/extras/mini-os/include/console.h
> index 3755b66..a77f47f 100644
> --- a/extras/mini-os/include/console.h
> +++ b/extras/mini-os/include/console.h
> @@ -64,8 +64,8 @@ struct consfront_dev {
>  
>  
>  void print(int direct, const char *fmt, va_list args);

What about a printf notation for this print function?

> -void printk(const char *fmt, ...);
> -void xprintk(const char *fmt, ...);
> +void printk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
> +void xprintk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
>  
>  #define tprintk(_fmt, _args...) printk("[%s] " _fmt, current->name, ##_args) 
>  
> diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
> index 13e7e59..1e9bddc 100644
> --- a/extras/mini-os/lib/sys.c
> +++ b/extras/mini-os/lib/sys.c
> @@ -1319,7 +1319,7 @@ int clock_gettime(clockid_t clk_id, struct timespec *tp)
>  	    break;
>  	}
>  	default:
> -	    print_unsupported("clock_gettime(%d)", clk_id);
> +	    print_unsupported("clock_gettime(%ld)", (long) clk_id);

I am failing to locate a type declaration for clockid_t, but I feel that
is is likely you can get away without a cast.

>  	    errno = EINVAL;
>  	    return -1;
>      }
> @@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
>          mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
>      }
>  
> -    printk("sparsing %ldMB at %lx\n", size >> 20, data);
> +    printk("sparsing %ldMB at %lx\n", (long) size >> 20, data);

%zu is the correct format for a size_t.

>  
>      munmap((void *) data, size);
>      free_physical_pages(mfns, n);
> diff --git a/extras/mini-os/lwip-arch.c b/extras/mini-os/lwip-arch.c
> index e634ef4..e2476d4 100644
> --- a/extras/mini-os/lwip-arch.c
> +++ b/extras/mini-os/lwip-arch.c
> @@ -236,7 +236,7 @@ sys_thread_t sys_thread_new(char *name, void (* thread)(void *arg), void *arg, i
>  {
>      struct thread *t;
>      if (stacksize > STACK_SIZE) {
> -	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, STACK_SIZE);
> +	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, (int) STACK_SIZE);

STACK_SIZE is an unsigned long.

>  	do_exit();
>      }
>      lwip_thread = t = create_thread(name, thread, arg);
> diff --git a/extras/mini-os/minios.mk b/extras/mini-os/minios.mk
> index f42f48b..20ba64b 100644
> --- a/extras/mini-os/minios.mk
> +++ b/extras/mini-os/minios.mk
> @@ -6,7 +6,7 @@ debug = y
>  
>  # Define some default flags.
>  # NB. '-Wcast-qual' is nasty, so I omitted it.
> -DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls
> +DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls -Wformat

Please don't add overrides while leaving the previous still present. 
Drop both the -Wno-format and -Wformat

(Unrelated to your patch, A -Wredundat-decls followed by
-Wno-redundant-decls seems silly)

~Andrew


>  DEF_CFLAGS += $(call cc-option,$(CC),-fno-stack-protector,)
>  DEF_CFLAGS += $(call cc-option,$(CC),-fgnu89-inline)
>  DEF_CFLAGS += -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline
> diff --git a/extras/mini-os/mm.c b/extras/mini-os/mm.c
> index 64b3292..31aaf83 100644
> --- a/extras/mini-os/mm.c
> +++ b/extras/mini-os/mm.c
> @@ -379,7 +379,11 @@ void *sbrk(ptrdiff_t increment)
>      unsigned long new_brk = old_brk + increment;
>  
>      if (new_brk > heap_end) {
> -	printk("Heap exhausted: %p + %lx = %p > %p\n", old_brk, increment, new_brk, heap_end);
> +	printk("Heap exhausted: %lx + %lx = %p > %p\n",
> +			old_brk,
> +			(unsigned long) increment,
> +			(void *) new_brk,
> +			(void *) heap_end);
>  	return NULL;
>      }
>      
> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> index 44c3995..6f335fe 100644
> --- a/extras/mini-os/netfront.c
> +++ b/extras/mini-os/netfront.c
> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
>      dev->fd = -1;
>  #endif
>  
> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
>      init_SEMAPHORE(&dev->tx_sem, NET_TX_RING_SIZE);
>      for(i=0;i<NET_TX_RING_SIZE;i++)
>      {
> diff --git a/extras/mini-os/sched.c b/extras/mini-os/sched.c
> index d0c607e..1e843d9 100644
> --- a/extras/mini-os/sched.c
> +++ b/extras/mini-os/sched.c
> @@ -276,7 +276,7 @@ void th_f2(void *data)
>  {
>      for(;;)
>      {
> -        printk("Thread OTHER executing, data 0x%lx\n", data);
> +        printk("Thread OTHER executing, data 0x%p\n", data);
>          schedule();
>      }
>  }
> diff --git a/extras/mini-os/test.c b/extras/mini-os/test.c
> index 20d372b..64a4057 100644
> --- a/extras/mini-os/test.c
> +++ b/extras/mini-os/test.c
> @@ -116,7 +116,7 @@ static void blk_read_completed(struct blkfront_aiocb *aiocb, int ret)
>  {
>      struct blk_req *req = aiocb->data;
>      if (ret)
> -        printk("got error code %d when reading at offset %ld\n", ret, aiocb->aio_offset);
> +        printk("got error code %d when reading at offset %ld\n", ret, (long) aiocb->aio_offset);
>      else
>          blk_size_read += blk_info.sector_size;
>      free(aiocb->aio_buf);
> @@ -237,7 +237,9 @@ static void blkfront_thread(void *p)
>          blkfront_aio_poll(blk_dev);
>          gettimeofday(&tv, NULL);
>          if (tv.tv_sec > lasttime + 10) {
> -            printk("%llu read, %llu write\n", blk_size_read, blk_size_write);
> +            printk("%llu read, %llu write\n",
> +                    (unsigned long long) blk_size_read,
> +                    (unsigned long long) blk_size_write);
>              lasttime = tv.tv_sec;
>          }
>  
> diff --git a/extras/mini-os/tpm_tis.c b/extras/mini-os/tpm_tis.c
> index dc4134a..936e854 100644
> --- a/extras/mini-os/tpm_tis.c
> +++ b/extras/mini-os/tpm_tis.c
> @@ -906,7 +906,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
>     //down(&chip->tpm_mutex);
>  
>     if ((rc = tpm_tis_send(chip, (uint8_t *) buf, count)) < 0) {
> -      printk("tpm_transmit: tpm_send: error %ld\n", rc);
> +      printk("tpm_transmit: tpm_send: error %ld\n", (long) rc);
>        goto out;
>     }
>  
> @@ -938,7 +938,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
>  
>  out_recv:
>     if((rc = tpm_tis_recv(chip, (uint8_t *) buf, bufsiz)) < 0) {
> -      printk("tpm_transmit: tpm_recv: error %d\n", rc);
> +      printk("tpm_transmit: tpm_recv: error %d\n", (int) rc);
>     }
>  out:
>     //up(&chip->tpm_mutex);
> @@ -977,7 +977,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  
>     if((rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>  	 "attempting to determine the timeouts")) != 0) {
> -      printk("transmit failed %d\n", rc);
> +      printk("transmit failed %d\n", (int) rc);
>        goto duration;
>     }
>  
> @@ -1132,7 +1132,7 @@ struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned i
>        if(locality_enabled(tpm, i)) {
>  	 /* Map the page in now */
>  	 if((tpm->pages[i] = ioremap_nocache(addr, PAGE_SIZE)) == NULL) {
> -	    printk("Unable to map iomem page a address %p\n", addr);
> +	    printk("Unable to map iomem page a address %lx\n", addr);
>  	    goto abort_egress;
>  	 }
>  
> diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c
> index 934f23b..4613ed6 100644
> --- a/extras/mini-os/xenbus/xenbus.c
> +++ b/extras/mini-os/xenbus/xenbus.c
> @@ -337,8 +337,8 @@ void init_xenbus(void)
>  		      xenbus_evtchn_handler,
>                NULL);
>      unmask_evtchn(start_info.store_evtchn);
> -    printk("xenbus initialised on irq %d mfn %#lx\n",
> -	   err, start_info.store_mfn);
> +    printk("xenbus initialised on irq %d mfn %#llx\n",
> +	   err, (unsigned long long) start_info.store_mfn);
>  }
>  
>  void fini_xenbus(void)
> diff --git a/stubdom/vtpmmgr/disk_read.c b/stubdom/vtpmmgr/disk_read.c
> index 33aacdd..a4c63c8 100644
> --- a/stubdom/vtpmmgr/disk_read.c
> +++ b/stubdom/vtpmmgr/disk_read.c
> @@ -538,7 +538,7 @@ int vtpm_load_disk(void)
>  	printk(" root seal: %lu; sector of %d: %lu\n",
>  		sizeof(struct disk_root_sealed_data), SEALS_PER_ROOT_SEAL_LIST, sizeof(struct disk_seal_list));
>  	printk(" root: %lu v=%lu\n", sizeof(root1), sizeof(root1.v));
> -	printk(" itree: %lu; sector of %d: %lu\n",
> +	printk(" itree: %u; sector of %d: %lu\n",
>  		4 + 32, NR_ENTRIES_PER_ITREE, sizeof(struct disk_itree_sector));
>  	printk(" group: %lu v=%lu id=%lu md=%lu\n",
>  		sizeof(struct disk_group_sector), sizeof(struct disk_group_sector_mac3_area),

Re: [PATCH] mini-os: enable compiler check for printk format types

[Jan Beulich]

>>> On 06.08.14 at 18:29, <andrew.cooper3@citrix.com> wrote:
> On 06/08/2014 10:44, Thomas Leonard wrote:
>> @@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
>>          mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
>>      }
>>  
>> -    printk("sparsing %ldMB at %lx\n", size >> 20, data);
>> +    printk("sparsing %ldMB at %lx\n", (long) size >> 20, data);
> 
> %zu is the correct format for a size_t.

If only mini-os'es vsnprintf() supported 'z' - interestingly vsscanf()
does, but vsnprintf() only knows about 'Z' right now.

Jan

Re: [PATCH] mini-os: enable compiler check for printk format types

[Samuel Thibault]

Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
> @@ -326,7 +326,7 @@ static void set_readonly(void *text, void *etext)
>              count++;
>          }
>          else
> -            printk("skipped %p\n", start_address);
> +            printk("skipped %lx\n", start_address);

Please prepend 0x, and likewise further down.

> @@ -369,21 +369,21 @@ int mem_test(unsigned long *start_va, unsigned long *end_va, int verbose)
>      /* write values and print page walks */
>      if ( verbose && (((unsigned long)start_va) & 0xfffff) )
>      {
> -        printk("MemTest Start: 0x%lx\n", start_va);
> +        printk("MemTest Start: 0x%p\n", start_va);

Please drop 0x, and likewise further down.

> @@ -516,7 +516,7 @@ void arch_init_demand_mapping_area(unsigned long cur_pfn)
>  
>      demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
>      cur_pfn += DEMAND_MAP_PAGES;
> -    printk("Demand map pfns at %lx-%lx.\n", 
> +    printk("Demand map pfns at %lx-%p.\n", 

Please prepend 0x to %lx too, to have coherency.

> @@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
>          mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
>      }
>  
> -    printk("sparsing %ldMB at %lx\n", size >> 20, data);
> +    printk("sparsing %ldMB at %lx\n", (long) size >> 20, data);

Please cast into long after the shift, not before.

> @@ -236,7 +236,7 @@ sys_thread_t sys_thread_new(char *name, void (* thread)(void *arg), void *arg, i
>  {
>      struct thread *t;
>      if (stacksize > STACK_SIZE) {
> -	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, STACK_SIZE);
> +	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, (int) STACK_SIZE);

Please rather cast to long and use %lu.

> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> index 44c3995..6f335fe 100644
> --- a/extras/mini-os/netfront.c
> +++ b/extras/mini-os/netfront.c
> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
>      dev->fd = -1;
>  #endif
>  
> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);

lib/printf.c does not actually support %ll yet, it uses %L instead.
We'd rather not write code using that until somebody fixes lib/printf.c.
Here casting to unsigned long and using %lu will be way enough anyway.

Samuel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Thomas Leonard]

[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]

On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
>> @@ -326,7 +326,7 @@ static void set_readonly(void *text, void *etext)
>>              count++;
>>          }
>>          else
>> -            printk("skipped %p\n", start_address);
>> +            printk("skipped %lx\n", start_address);
>
> Please prepend 0x, and likewise further down.

Mini-OS's printk doesn't add a 0x prefix for either %p or %lx, so
doing this would change the behaviour (which might be good, but I
don't think it should be part of this patch, which is only about
fixing the types).

>> @@ -369,21 +369,21 @@ int mem_test(unsigned long *start_va, unsigned long *end_va, int verbose)
>>      /* write values and print page walks */
>>      if ( verbose && (((unsigned long)start_va) & 0xfffff) )
>>      {
>> -        printk("MemTest Start: 0x%lx\n", start_va);
>> +        printk("MemTest Start: 0x%p\n", start_va);
>
> Please drop 0x, and likewise further down.
>
>> @@ -516,7 +516,7 @@ void arch_init_demand_mapping_area(unsigned long cur_pfn)
>>
>>      demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
>>      cur_pfn += DEMAND_MAP_PAGES;
>> -    printk("Demand map pfns at %lx-%lx.\n",
>> +    printk("Demand map pfns at %lx-%p.\n",
>
> Please prepend 0x to %lx too, to have coherency.
>
>> @@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
>>          mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
>>      }
>>
>> -    printk("sparsing %ldMB at %lx\n", size >> 20, data);
>> +    printk("sparsing %ldMB at %lx\n", (long) size >> 20, data);
>
> Please cast into long after the shift, not before.

Fixed.

>> @@ -236,7 +236,7 @@ sys_thread_t sys_thread_new(char *name, void (* thread)(void *arg), void *arg, i
>>  {
>>      struct thread *t;
>>      if (stacksize > STACK_SIZE) {
>> -     printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, STACK_SIZE);
>> +     printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, (int) STACK_SIZE);
>
> Please rather cast to long and use %lu.

OK.

>> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
>> index 44c3995..6f335fe 100644
>> --- a/extras/mini-os/netfront.c
>> +++ b/extras/mini-os/netfront.c
>> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
>>      dev->fd = -1;
>>  #endif
>>
>> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
>> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
>> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
>> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
>
> lib/printf.c does not actually support %ll yet, it uses %L instead.
> We'd rather not write code using that until somebody fixes lib/printf.c.
> Here casting to unsigned long and using %lu will be way enough anyway.

Fixed.


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

[-- Attachment #2: 0001-mini-os-enable-compiler-check-for-printk-format-type.patch --]
[-- Type: text/x-patch, Size: 17130 bytes --]

From 52ff3744ac817df9008a395c552a65c8d4db19de Mon Sep 17 00:00:00 2001
From: Thomas Leonard <talex5@gmail.com>
Date: Wed, 6 Aug 2014 09:25:20 +0100
Subject: [PATCH] mini-os: enable compiler check for printk format types

Signed-off-by: Thomas Leonard <talex5@gmail.com>

---

- cast after >>, not before
- format STACK_SIZE as long
- avoid using %llu (which isn't implemented)
---
 extras/mini-os/arch/x86/mm.c     | 22 +++++++++++-----------
 extras/mini-os/arch/x86/sched.c  |  4 ++--
 extras/mini-os/arch/x86/traps.c  | 18 +++++++++---------
 extras/mini-os/blkfront.c        |  2 +-
 extras/mini-os/include/console.h |  4 ++--
 extras/mini-os/lib/sys.c         |  4 ++--
 extras/mini-os/lwip-arch.c       |  5 +++--
 extras/mini-os/minios.mk         |  2 +-
 extras/mini-os/mm.c              |  6 +++++-
 extras/mini-os/netfront.c        |  4 ++--
 extras/mini-os/sched.c           |  2 +-
 extras/mini-os/test.c            |  6 ++++--
 extras/mini-os/tpm_tis.c         |  8 ++++----
 extras/mini-os/xenbus/xenbus.c   |  4 ++--
 stubdom/vtpmmgr/disk_read.c      |  2 +-
 15 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/extras/mini-os/arch/x86/mm.c b/extras/mini-os/arch/x86/mm.c
index 9c6d1b8..186659d 100644
--- a/extras/mini-os/arch/x86/mm.c
+++ b/extras/mini-os/arch/x86/mm.c
@@ -94,7 +94,7 @@ static void new_pt_frame(unsigned long *pt_pfn, unsigned long prev_l_mfn,
         break;
 #endif
     default:
-        printk("new_pt_frame() called with invalid level number %d\n", level);
+        printk("new_pt_frame() called with invalid level number %lu\n", level);
         do_exit();
         break;
     }
@@ -181,7 +181,7 @@ static int need_pt_frame(unsigned long va, int level)
         if ( level == L1_FRAME )
             return 1;
 
-    printk("ERROR: Unknown frame level %d, hypervisor %llx,%llx\n", 
+    printk("ERROR: Unknown frame level %d, hypervisor %lx,%lx\n", 
            level, hyp_virt_start, hyp_virt_end);
     return -1;
 }
@@ -206,11 +206,11 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
     if ( *max_pfn >= virt_to_pfn(HYPERVISOR_VIRT_START) )
     {
         printk("WARNING: Mini-OS trying to use Xen virtual space. "
-               "Truncating memory from %dMB to ",
+               "Truncating memory from %luMB to ",
                ((unsigned long)pfn_to_virt(*max_pfn) -
                 (unsigned long)&_text)>>20);
         *max_pfn = virt_to_pfn(HYPERVISOR_VIRT_START - PAGE_SIZE);
-        printk("%dMB\n",
+        printk("%luMB\n",
                ((unsigned long)pfn_to_virt(*max_pfn) - 
                 (unsigned long)&_text)>>20);
     }
@@ -326,7 +326,7 @@ static void set_readonly(void *text, void *etext)
             count++;
         }
         else
-            printk("skipped %p\n", start_address);
+            printk("skipped %lx\n", start_address);
 
         start_address += PAGE_SIZE;
 
@@ -369,21 +369,21 @@ int mem_test(unsigned long *start_va, unsigned long *end_va, int verbose)
     /* write values and print page walks */
     if ( verbose && (((unsigned long)start_va) & 0xfffff) )
     {
-        printk("MemTest Start: 0x%lx\n", start_va);
+        printk("MemTest Start: 0x%p\n", start_va);
         page_walk((unsigned long)start_va);
     }
     for ( pointer = start_va; pointer < end_va; pointer++ )
     {
         if ( verbose && !(((unsigned long)pointer) & 0xfffff) )
         {
-            printk("Writing to %lx\n", pointer);
+            printk("Writing to %p\n", pointer);
             page_walk((unsigned long)pointer);
         }
         *pointer = (unsigned long)pointer & ~mask;
     }
     if ( verbose && (((unsigned long)end_va) & 0xfffff) )
     {
-        printk("MemTest End: %lx\n", end_va-1);
+        printk("MemTest End: %p\n", end_va-1);
         page_walk((unsigned long)end_va-1);
     }
  
@@ -516,7 +516,7 @@ void arch_init_demand_mapping_area(unsigned long cur_pfn)
 
     demand_map_area_start = (unsigned long) pfn_to_virt(cur_pfn);
     cur_pfn += DEMAND_MAP_PAGES;
-    printk("Demand map pfns at %lx-%lx.\n", 
+    printk("Demand map pfns at %lx-%p.\n", 
            demand_map_area_start, pfn_to_virt(cur_pfn));
 
 #ifdef HAVE_LIBC
@@ -619,7 +619,7 @@ void do_map_frames(unsigned long va,
                 if (err)
                     err[done * stride] = rc;
                 else {
-                    printk("Map %ld (%lx, ...) at %p failed: %d.\n",
+                    printk("Map %ld (%lx, ...) at %lx failed: %d.\n",
                            todo, mfns[done * stride] + done * incr, va, rc);
                     do_exit();
                 }
@@ -793,7 +793,7 @@ unsigned long alloc_contig_pages(int order, unsigned int addr_bits)
     out_frames = virt_to_pfn(in_va); /* PFNs to populate */
     ret = HYPERVISOR_memory_op(XENMEM_exchange, &exchange);
     if ( ret ) {
-        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%d\n", 
+        printk("mem exchanged order=0x%x failed with rc=%d, nr_exchanged=%lu\n",
                order, ret, exchange.nr_exchanged);
         /* we still need to return the allocated pages above to the pool
          * ie. map them back into the 1:1 mapping etc. so we continue but 
diff --git a/extras/mini-os/arch/x86/sched.c b/extras/mini-os/arch/x86/sched.c
index e4a3dc2..ec13694 100644
--- a/extras/mini-os/arch/x86/sched.c
+++ b/extras/mini-os/arch/x86/sched.c
@@ -73,7 +73,7 @@ void dump_stack(struct thread *thread)
     printk("The stack for \"%s\"\n", thread->name);
     for(count = 0; count < 25 && pointer < bottom; count ++)
     {
-        printk("[0x%lx] 0x%lx\n", pointer, *pointer);
+        printk("[0x%p] 0x%lx\n", pointer, *pointer);
         pointer++;
     }
     
@@ -101,7 +101,7 @@ struct thread* arch_create_thread(char *name, void (*function)(void *),
     /* We can't use lazy allocation here since the trap handler runs on the stack */
     thread->stack = (char *)alloc_pages(STACK_SIZE_PAGE_ORDER);
     thread->name = name;
-    printk("Thread \"%s\": pointer: 0x%lx, stack: 0x%lx\n", name, thread, 
+    printk("Thread \"%s\": pointer: 0x%p, stack: 0x%p\n", name, thread, 
             thread->stack);
     
     thread->sp = (unsigned long)thread->stack + STACK_SIZE;
diff --git a/extras/mini-os/arch/x86/traps.c b/extras/mini-os/arch/x86/traps.c
index 516d133..6353718 100644
--- a/extras/mini-os/arch/x86/traps.c
+++ b/extras/mini-os/arch/x86/traps.c
@@ -34,14 +34,14 @@ void dump_regs(struct pt_regs *regs)
 {
     printk("Thread: %s\n", current->name);
 #ifdef __i386__    
-    printk("EIP: %x, EFLAGS %x.\n", regs->eip, regs->eflags);
-    printk("EBX: %08x ECX: %08x EDX: %08x\n",
+    printk("EIP: %lx, EFLAGS %lx.\n", regs->eip, regs->eflags);
+    printk("EBX: %08lx ECX: %08lx EDX: %08lx\n",
 	   regs->ebx, regs->ecx, regs->edx);
-    printk("ESI: %08x EDI: %08x EBP: %08x EAX: %08x\n",
+    printk("ESI: %08lx EDI: %08lx EBP: %08lx EAX: %08lx\n",
 	   regs->esi, regs->edi, regs->ebp, regs->eax);
-    printk("DS: %04x ES: %04x orig_eax: %08x, eip: %08x\n",
+    printk("DS: %04x ES: %04x orig_eax: %08lx, eip: %08lx\n",
 	   regs->xds, regs->xes, regs->orig_eax, regs->eip);
-    printk("CS: %04x EFLAGS: %08x esp: %08x ss: %04x\n",
+    printk("CS: %04x EFLAGS: %08lx esp: %08lx ss: %04x\n",
 	   regs->xcs, regs->eflags, regs->esp, regs->xss);
 #else
     printk("RIP: %04lx:[<%016lx>] ", regs->cs & 0xffff, regs->rip);
@@ -214,10 +214,10 @@ void do_page_fault(struct pt_regs *regs, unsigned long error_code)
     barrier();
 
 #if defined(__x86_64__)
-    printk("Page fault at linear address %p, rip %p, regs %p, sp %p, our_sp %p, code %lx\n",
+    printk("Page fault at linear address %lx, rip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
            addr, regs->rip, regs, regs->rsp, &addr, error_code);
 #else
-    printk("Page fault at linear address %p, eip %p, regs %p, sp %p, our_sp %p, code %lx\n",
+    printk("Page fault at linear address %lx, eip %lx, regs %p, sp %lx, our_sp %p, code %lx\n",
            addr, regs->eip, regs, regs->esp, &addr, error_code);
 #endif
 
@@ -243,9 +243,9 @@ void do_general_protection(struct pt_regs *regs, long error_code)
 {
     struct sched_shutdown sched_shutdown = { .reason = SHUTDOWN_crash };
 #ifdef __i386__
-    printk("GPF eip: %p, error_code=%lx\n", regs->eip, error_code);
+    printk("GPF eip: %lx, error_code=%lx\n", regs->eip, error_code);
 #else    
-    printk("GPF rip: %p, error_code=%lx\n", regs->rip, error_code);
+    printk("GPF rip: %lx, error_code=%lx\n", regs->rip, error_code);
 #endif
     dump_regs(regs);
 #if defined(__x86_64__)
diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c
index 59e576f..bdb7765 100644
--- a/extras/mini-os/blkfront.c
+++ b/extras/mini-os/blkfront.c
@@ -236,7 +236,7 @@ done:
     }
     unmask_evtchn(dev->evtchn);
 
-    printk("%u sectors of %u bytes\n", dev->info.sectors, dev->info.sector_size);
+    printk("%lu sectors of %u bytes\n", (unsigned long) dev->info.sectors, dev->info.sector_size);
     printk("**************************\n");
 
     return dev;
diff --git a/extras/mini-os/include/console.h b/extras/mini-os/include/console.h
index 3755b66..a77f47f 100644
--- a/extras/mini-os/include/console.h
+++ b/extras/mini-os/include/console.h
@@ -64,8 +64,8 @@ struct consfront_dev {
 
 
 void print(int direct, const char *fmt, va_list args);
-void printk(const char *fmt, ...);
-void xprintk(const char *fmt, ...);
+void printk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
+void xprintk(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
 
 #define tprintk(_fmt, _args...) printk("[%s] " _fmt, current->name, ##_args) 
 
diff --git a/extras/mini-os/lib/sys.c b/extras/mini-os/lib/sys.c
index 13e7e59..f8d7a7c 100644
--- a/extras/mini-os/lib/sys.c
+++ b/extras/mini-os/lib/sys.c
@@ -1319,7 +1319,7 @@ int clock_gettime(clockid_t clk_id, struct timespec *tp)
 	    break;
 	}
 	default:
-	    print_unsupported("clock_gettime(%d)", clk_id);
+	    print_unsupported("clock_gettime(%ld)", (long) clk_id);
 	    errno = EINVAL;
 	    return -1;
     }
@@ -1421,7 +1421,7 @@ void sparse(unsigned long data, size_t size)
         mfns[i] = virtual_to_mfn(data + i * PAGE_SIZE);
     }
 
-    printk("sparsing %ldMB at %lx\n", size >> 20, data);
+    printk("sparsing %ldMB at %lx\n", ((long) size) >> 20, data);
 
     munmap((void *) data, size);
     free_physical_pages(mfns, n);
diff --git a/extras/mini-os/lwip-arch.c b/extras/mini-os/lwip-arch.c
index e634ef4..154617b 100644
--- a/extras/mini-os/lwip-arch.c
+++ b/extras/mini-os/lwip-arch.c
@@ -236,8 +236,9 @@ sys_thread_t sys_thread_new(char *name, void (* thread)(void *arg), void *arg, i
 {
     struct thread *t;
     if (stacksize > STACK_SIZE) {
-	printk("Can't start lwIP thread: stack size %d is too large for our %d\n", stacksize, STACK_SIZE);
-	do_exit();
+        printk("Can't start lwIP thread: stack size %d is too large for our %lu\n",
+                stacksize, (unsigned long) STACK_SIZE);
+        do_exit();
     }
     lwip_thread = t = create_thread(name, thread, arg);
     return t;
diff --git a/extras/mini-os/minios.mk b/extras/mini-os/minios.mk
index f42f48b..20ba64b 100644
--- a/extras/mini-os/minios.mk
+++ b/extras/mini-os/minios.mk
@@ -6,7 +6,7 @@ debug = y
 
 # Define some default flags.
 # NB. '-Wcast-qual' is nasty, so I omitted it.
-DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls
+DEF_CFLAGS += -fno-builtin -Wall -Werror -Wredundant-decls -Wno-format -Wno-redundant-decls -Wformat
 DEF_CFLAGS += $(call cc-option,$(CC),-fno-stack-protector,)
 DEF_CFLAGS += $(call cc-option,$(CC),-fgnu89-inline)
 DEF_CFLAGS += -Wstrict-prototypes -Wnested-externs -Wpointer-arith -Winline
diff --git a/extras/mini-os/mm.c b/extras/mini-os/mm.c
index 64b3292..31aaf83 100644
--- a/extras/mini-os/mm.c
+++ b/extras/mini-os/mm.c
@@ -379,7 +379,11 @@ void *sbrk(ptrdiff_t increment)
     unsigned long new_brk = old_brk + increment;
 
     if (new_brk > heap_end) {
-	printk("Heap exhausted: %p + %lx = %p > %p\n", old_brk, increment, new_brk, heap_end);
+	printk("Heap exhausted: %lx + %lx = %p > %p\n",
+			old_brk,
+			(unsigned long) increment,
+			(void *) new_brk,
+			(void *) heap_end);
 	return NULL;
     }
     
diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
index 44c3995..4e94997 100644
--- a/extras/mini-os/netfront.c
+++ b/extras/mini-os/netfront.c
@@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
     dev->fd = -1;
 #endif
 
-    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
-    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
+    printk("net TX ring size %lu\n", (unsigned long) NET_TX_RING_SIZE);
+    printk("net RX ring size %lu\n", (unsigned long) NET_RX_RING_SIZE);
     init_SEMAPHORE(&dev->tx_sem, NET_TX_RING_SIZE);
     for(i=0;i<NET_TX_RING_SIZE;i++)
     {
diff --git a/extras/mini-os/sched.c b/extras/mini-os/sched.c
index d0c607e..1e843d9 100644
--- a/extras/mini-os/sched.c
+++ b/extras/mini-os/sched.c
@@ -276,7 +276,7 @@ void th_f2(void *data)
 {
     for(;;)
     {
-        printk("Thread OTHER executing, data 0x%lx\n", data);
+        printk("Thread OTHER executing, data 0x%p\n", data);
         schedule();
     }
 }
diff --git a/extras/mini-os/test.c b/extras/mini-os/test.c
index 20d372b..64a4057 100644
--- a/extras/mini-os/test.c
+++ b/extras/mini-os/test.c
@@ -116,7 +116,7 @@ static void blk_read_completed(struct blkfront_aiocb *aiocb, int ret)
 {
     struct blk_req *req = aiocb->data;
     if (ret)
-        printk("got error code %d when reading at offset %ld\n", ret, aiocb->aio_offset);
+        printk("got error code %d when reading at offset %ld\n", ret, (long) aiocb->aio_offset);
     else
         blk_size_read += blk_info.sector_size;
     free(aiocb->aio_buf);
@@ -237,7 +237,9 @@ static void blkfront_thread(void *p)
         blkfront_aio_poll(blk_dev);
         gettimeofday(&tv, NULL);
         if (tv.tv_sec > lasttime + 10) {
-            printk("%llu read, %llu write\n", blk_size_read, blk_size_write);
+            printk("%llu read, %llu write\n",
+                    (unsigned long long) blk_size_read,
+                    (unsigned long long) blk_size_write);
             lasttime = tv.tv_sec;
         }
 
diff --git a/extras/mini-os/tpm_tis.c b/extras/mini-os/tpm_tis.c
index dc4134a..936e854 100644
--- a/extras/mini-os/tpm_tis.c
+++ b/extras/mini-os/tpm_tis.c
@@ -906,7 +906,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
    //down(&chip->tpm_mutex);
 
    if ((rc = tpm_tis_send(chip, (uint8_t *) buf, count)) < 0) {
-      printk("tpm_transmit: tpm_send: error %ld\n", rc);
+      printk("tpm_transmit: tpm_send: error %ld\n", (long) rc);
       goto out;
    }
 
@@ -938,7 +938,7 @@ static ssize_t tpm_transmit(struct tpm_chip *chip, const uint8_t *buf,
 
 out_recv:
    if((rc = tpm_tis_recv(chip, (uint8_t *) buf, bufsiz)) < 0) {
-      printk("tpm_transmit: tpm_recv: error %d\n", rc);
+      printk("tpm_transmit: tpm_recv: error %d\n", (int) rc);
    }
 out:
    //up(&chip->tpm_mutex);
@@ -977,7 +977,7 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 
    if((rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
 	 "attempting to determine the timeouts")) != 0) {
-      printk("transmit failed %d\n", rc);
+      printk("transmit failed %d\n", (int) rc);
       goto duration;
    }
 
@@ -1132,7 +1132,7 @@ struct tpm_chip* init_tpm_tis(unsigned long baseaddr, int localities, unsigned i
       if(locality_enabled(tpm, i)) {
 	 /* Map the page in now */
 	 if((tpm->pages[i] = ioremap_nocache(addr, PAGE_SIZE)) == NULL) {
-	    printk("Unable to map iomem page a address %p\n", addr);
+	    printk("Unable to map iomem page a address %lx\n", addr);
 	    goto abort_egress;
 	 }
 
diff --git a/extras/mini-os/xenbus/xenbus.c b/extras/mini-os/xenbus/xenbus.c
index 934f23b..4613ed6 100644
--- a/extras/mini-os/xenbus/xenbus.c
+++ b/extras/mini-os/xenbus/xenbus.c
@@ -337,8 +337,8 @@ void init_xenbus(void)
 		      xenbus_evtchn_handler,
               NULL);
     unmask_evtchn(start_info.store_evtchn);
-    printk("xenbus initialised on irq %d mfn %#lx\n",
-	   err, start_info.store_mfn);
+    printk("xenbus initialised on irq %d mfn %#llx\n",
+	   err, (unsigned long long) start_info.store_mfn);
 }
 
 void fini_xenbus(void)
diff --git a/stubdom/vtpmmgr/disk_read.c b/stubdom/vtpmmgr/disk_read.c
index 33aacdd..a4c63c8 100644
--- a/stubdom/vtpmmgr/disk_read.c
+++ b/stubdom/vtpmmgr/disk_read.c
@@ -538,7 +538,7 @@ int vtpm_load_disk(void)
 	printk(" root seal: %lu; sector of %d: %lu\n",
 		sizeof(struct disk_root_sealed_data), SEALS_PER_ROOT_SEAL_LIST, sizeof(struct disk_seal_list));
 	printk(" root: %lu v=%lu\n", sizeof(root1), sizeof(root1.v));
-	printk(" itree: %lu; sector of %d: %lu\n",
+	printk(" itree: %u; sector of %d: %lu\n",
 		4 + 32, NR_ENTRIES_PER_ITREE, sizeof(struct disk_itree_sector));
 	printk(" group: %lu v=%lu id=%lu md=%lu\n",
 		sizeof(struct disk_group_sector), sizeof(struct disk_group_sector_mac3_area),
-- 
2.0.3


[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Thomas Leonard]

On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
[...]
>> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
>> index 44c3995..6f335fe 100644
>> --- a/extras/mini-os/netfront.c
>> +++ b/extras/mini-os/netfront.c
>> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
>>      dev->fd = -1;
>>  #endif
>>
>> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
>> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
>> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
>> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
>
> lib/printf.c does not actually support %ll yet, it uses %L instead.

Actually, it looks like it does:

if (qualifier == 'l' && *fmt == 'l') {
  qualifier = 'L';
  ++fmt;
}

https://github.com/talex5/xen/blob/f71a229144d1316ff52fc8f1229c7c70573f44e2/extras/mini-os/lib/printf.c#L352


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Samuel Thibault]

Thomas Leonard, le Fri 08 Aug 2014 15:28:22 +0100, a écrit :
> On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
> [...]
> >> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> >> index 44c3995..6f335fe 100644
> >> --- a/extras/mini-os/netfront.c
> >> +++ b/extras/mini-os/netfront.c
> >> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
> >>      dev->fd = -1;
> >>  #endif
> >>
> >> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> >> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> >> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> >> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
> >
> > lib/printf.c does not actually support %ll yet, it uses %L instead.
> 
> Actually, it looks like it does:
> 
> if (qualifier == 'l' && *fmt == 'l') {
>   qualifier = 'L';
>   ++fmt;
> }

Oh, I missed this part of the code, thanks. For this exerpt of code,
long long is way too much, but for things like disk offsets and such,
it'd be preferrable to go for long long.

Samuel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Thomas Leonard]

On 8 August 2014 15:32, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> Thomas Leonard, le Fri 08 Aug 2014 15:28:22 +0100, a écrit :
>> On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
>> > Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
>> [...]
>> >> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
>> >> index 44c3995..6f335fe 100644
>> >> --- a/extras/mini-os/netfront.c
>> >> +++ b/extras/mini-os/netfront.c
>> >> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
>> >>      dev->fd = -1;
>> >>  #endif
>> >>
>> >> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
>> >> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
>> >> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
>> >> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
>> >
>> > lib/printf.c does not actually support %ll yet, it uses %L instead.
>>
>> Actually, it looks like it does:
>>
>> if (qualifier == 'l' && *fmt == 'l') {
>>   qualifier = 'L';
>>   ++fmt;
>> }
>
> Oh, I missed this part of the code, thanks. For this exerpt of code,
> long long is way too much, but for things like disk offsets and such,
> it'd be preferrable to go for long long.

I see this patch never go applied. To be clear: the above comment was
just for information. I agree with using %lu in the patch (as the
latest version of the patch does).

So, I think this is ready to be applied:

http://lists.xenproject.org/archives/html/xen-devel/2014-08/msg00998.html


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Ian Campbell]

On Fri, 2014-10-03 at 09:44 +0100, Thomas Leonard wrote:
> On 8 August 2014 15:32, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > Thomas Leonard, le Fri 08 Aug 2014 15:28:22 +0100, a écrit :
> >> On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> >> > Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
> >> [...]
> >> >> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> >> >> index 44c3995..6f335fe 100644
> >> >> --- a/extras/mini-os/netfront.c
> >> >> +++ b/extras/mini-os/netfront.c
> >> >> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
> >> >>      dev->fd = -1;
> >> >>  #endif
> >> >>
> >> >> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> >> >> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> >> >> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> >> >> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
> >> >
> >> > lib/printf.c does not actually support %ll yet, it uses %L instead.
> >>
> >> Actually, it looks like it does:
> >>
> >> if (qualifier == 'l' && *fmt == 'l') {
> >>   qualifier = 'L';
> >>   ++fmt;
> >> }
> >
> > Oh, I missed this part of the code, thanks. For this exerpt of code,
> > long long is way too much, but for things like disk offsets and such,
> > it'd be preferrable to go for long long.
> 
> I see this patch never go applied. To be clear: the above comment was
> just for information. I agree with using %lu in the patch (as the
> latest version of the patch does).
> 
> So, I think this is ready to be applied:
> 
> http://lists.xenproject.org/archives/html/xen-devel/2014-08/msg00998.html

Samuel, are you ok with the patch after those clarifications?

Ian.




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Samuel Thibault]

Ian Campbell, le Fri 03 Oct 2014 09:55:15 +0100, a écrit :
> On Fri, 2014-10-03 at 09:44 +0100, Thomas Leonard wrote:
> > On 8 August 2014 15:32, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > > Thomas Leonard, le Fri 08 Aug 2014 15:28:22 +0100, a écrit :
> > >> On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > >> > Thomas Leonard, le Wed 06 Aug 2014 10:44:00 +0100, a écrit :
> > >> [...]
> > >> >> diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c
> > >> >> index 44c3995..6f335fe 100644
> > >> >> --- a/extras/mini-os/netfront.c
> > >> >> +++ b/extras/mini-os/netfront.c
> > >> >> @@ -327,8 +327,8 @@ struct netfront_dev *init_netfront(char *_nodename, void (*thenetif_rx)(unsigned
> > >> >>      dev->fd = -1;
> > >> >>  #endif
> > >> >>
> > >> >> -    printk("net TX ring size %d\n", NET_TX_RING_SIZE);
> > >> >> -    printk("net RX ring size %d\n", NET_RX_RING_SIZE);
> > >> >> +    printk("net TX ring size %llu\n", (unsigned long long) NET_TX_RING_SIZE);
> > >> >> +    printk("net RX ring size %llu\n", (unsigned long long) NET_RX_RING_SIZE);
> > >> >
> > >> > lib/printf.c does not actually support %ll yet, it uses %L instead.
> > >>
> > >> Actually, it looks like it does:
> > >>
> > >> if (qualifier == 'l' && *fmt == 'l') {
> > >>   qualifier = 'L';
> > >>   ++fmt;
> > >> }
> > >
> > > Oh, I missed this part of the code, thanks. For this exerpt of code,
> > > long long is way too much, but for things like disk offsets and such,
> > > it'd be preferrable to go for long long.
> > 
> > I see this patch never go applied. To be clear: the above comment was
> > just for information. I agree with using %lu in the patch (as the
> > latest version of the patch does).
> > 
> > So, I think this is ready to be applied:
> > 
> > http://lists.xenproject.org/archives/html/xen-devel/2014-08/msg00998.html
> 
> Samuel, are you ok with the patch after those clarifications?

Yes.

Reviewing it just kept being behind loads of stuff in my TODO list :/

Samuel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Ian Campbell]

On Sun, 2014-10-05 at 21:30 +0200, Samuel Thibault wrote:

> > Samuel, are you ok with the patch after those clarifications?
> 
> Yes.

I interpreted this as an Acked-by, I hope that's ok, and was about to
commit when:
disk_read.c: In function 'vtpm_load_disk':
disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
disk_read.c:542:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'unsigned int' [-Werror=format]
disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]

That's on an x86_32 stubdom build.

Looks like this is printing the output of sizeof. The correct modifier
for printing a size_t is %zu not hardcoded as a long. I haven't checked
if mini-os printf supports z.

> Reviewing it just kept being behind loads of stuff in my TODO list :/

I know the feeling :-/

Re: [PATCH] mini-os: enable compiler check for printk format types

[Samuel Thibault]

Ian Campbell, le Mon 06 Oct 2014 14:28:50 +0100, a écrit :
> Looks like this is printing the output of sizeof. The correct modifier
> for printing a size_t is %zu not hardcoded as a long. I haven't checked
> if mini-os printf supports z.

Yes it does, so it can be used.

Samuel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Thomas Leonard]

On 6 October 2014 14:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Sun, 2014-10-05 at 21:30 +0200, Samuel Thibault wrote:
>
>> > Samuel, are you ok with the patch after those clarifications?
>>
>> Yes.
>
> I interpreted this as an Acked-by, I hope that's ok, and was about to
> commit when:
> disk_read.c: In function 'vtpm_load_disk':
> disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> disk_read.c:542:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'unsigned int' [-Werror=format]
> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
>
> That's on an x86_32 stubdom build.

What's the exact command I should use to test the builds? I did a
"make build" in "stubdom" in an x86_32 container and it worked*, but I
now see that doing "make" in the "stubdom/vtpmmgr" subdirectory gives
the above error.

* (after adding some symlinks to fix "xen-config-host.h:19:21: fatal
error: xenctrl.h: No such file or directory" error in "ioemu")

On 7 October 2014 17:04, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> Ian Campbell, le Mon 06 Oct 2014 14:28:50 +0100, a écrit :
>> Looks like this is printing the output of sizeof. The correct modifier
>> for printing a size_t is %zu not hardcoded as a long. I haven't checked
>> if mini-os printf supports z.
>
> Yes it does, so it can be used.

It will also warn about a %lu that should be just %u in there.
With those changes, vtpmmgr builds for me.


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] mini-os: enable compiler check for printk format types

[Andrew Cooper]

On 08/10/14 12:12, Thomas Leonard wrote:
> On 6 October 2014 14:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Sun, 2014-10-05 at 21:30 +0200, Samuel Thibault wrote:
>>
>>>> Samuel, are you ok with the patch after those clarifications?
>>> Yes.
>> I interpreted this as an Acked-by, I hope that's ok, and was about to
>> commit when:
>> disk_read.c: In function 'vtpm_load_disk':
>> disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
>> disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
>> disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
>> disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
>> disk_read.c:542:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
>> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
>> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
>> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
>> disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
>> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
>> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
>> disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'unsigned int' [-Werror=format]
>> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
>> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
>> disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
>>
>> That's on an x86_32 stubdom build.
> What's the exact command I should use to test the builds? I did a
> "make build" in "stubdom" in an x86_32 container and it worked*, but I
> now see that doing "make" in the "stubdom/vtpmmgr" subdirectory gives
> the above error.


make XEN_TARGET_ARCH=x86_32 <mumble>

>
> * (after adding some symlinks to fix "xen-config-host.h:19:21: fatal
> error: xenctrl.h: No such file or directory" error in "ioemu")

You need to refresh your underlying qemu-trad repository (`make -C tools
subtree-force-update` ought to work).  The header files moved, requiring
a synchronised update to both repositories.


~Andrew

Re: [PATCH] mini-os: enable compiler check for printk format types

[Ian Campbell]

On Wed, 2014-10-08 at 12:12 +0100, Thomas Leonard wrote:
> On 6 October 2014 14:28, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Sun, 2014-10-05 at 21:30 +0200, Samuel Thibault wrote:
> >
> >> > Samuel, are you ok with the patch after those clarifications?
> >>
> >> Yes.
> >
> > I interpreted this as an Acked-by, I hope that's ok, and was about to
> > commit when:
> > disk_read.c: In function 'vtpm_load_disk':
> > disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> > disk_read.c:539:59: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> > disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> > disk_read.c:540:2: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> > disk_read.c:542:30: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> > disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> > disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> > disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> > disk_read.c:545:47: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
> > disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> > disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Werror=format]
> > disk_read.c:548:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 6 has type 'unsigned int' [-Werror=format]
> > disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'unsigned int' [-Werror=format]
> > disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int' [-Werror=format]
> > disk_read.c:551:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Werror=format]
> >
> > That's on an x86_32 stubdom build.
> 
> What's the exact command I should use to test the builds? I did a
> "make build" in "stubdom" in an x86_32 container and it worked*, but I
> now see that doing "make" in the "stubdom/vtpmmgr" subdirectory gives
> the above error.

My scripts just do "make dist" in a 32-bit environment. I think make
"dist XEN_TARGET_ARCH=x86_32" should be approximately the same, although
I think there is some small amount stuff which isn't "cross" built.

Probably dist-stubdom (or is it stubdom-dist?) would do just the bit you
wanted.

> * (after adding some symlinks to fix "xen-config-host.h:19:21: fatal
> error: xenctrl.h: No such file or directory" error in "ioemu")

As Andy says you need to update your qemu. There are some force-update
targets in tools/Makefile which can help (or just nuke the trees)

Ian.