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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Wed, 2015-06-17 at 11:08 +0100, Thomas Leonard wrote:
> Signed-off-by: Thomas Leonard <talex5@xxxxxxxxx>
> ---
> Note: This patch might flag up warnings in downstream projects that use
> incorrect format codes. In particular, it doesn't include the patch to
> Xen's vtpm_load_disk, since that's now in a separate repository. See:
> 
> http://lists.xenproject.org/archives/html/xen-devel/2014-10/msg00899.html

Does such a fixup patch exist somewhere or are you intending to supply
one?

> ---
>  arch/x86/mm.c     | 22 +++++++++++-----------
>  arch/x86/sched.c  |  4 ++--
>  arch/x86/traps.c  | 18 +++++++++---------
>  blkfront.c        |  2 +-
>  include/console.h |  4 ++--
>  lib/sys.c         |  4 ++--
>  lwip-arch.c       |  5 +++--
>  minios.mk         |  2 +-
>  mm.c              |  6 +++++-
>  netfront.c        |  4 ++--
>  sched.c           |  2 +-
>  test.c            |  6 ++++--
>  tpm_tis.c         |  8 ++++----
>  xenbus/xenbus.c   |  4 ++--
>  14 files changed, 49 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/x86/mm.c b/arch/x86/mm.c
> index 9c6d1b8..186659d 100644
> --- a/arch/x86/mm.c
> +++ b/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/arch/x86/sched.c b/arch/x86/sched.c
> index e4a3dc2..ec13694 100644
> --- a/arch/x86/sched.c
> +++ b/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/arch/x86/traps.c b/arch/x86/traps.c
> index 516d133..6353718 100644
> --- a/arch/x86/traps.c
> +++ b/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/blkfront.c b/blkfront.c
> index 59e576f..bdb7765 100644
> --- a/blkfront.c
> +++ b/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/include/console.h b/include/console.h
> index 3755b66..a77f47f 100644
> --- a/include/console.h
> +++ b/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/lib/sys.c b/lib/sys.c
> index 13e7e59..f8d7a7c 100644
> --- a/lib/sys.c
> +++ b/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/lwip-arch.c b/lwip-arch.c
> index e634ef4..154617b 100644
> --- a/lwip-arch.c
> +++ b/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/minios.mk b/minios.mk
> index b0d9f71..e042027 100644
> --- a/minios.mk
> +++ b/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/mm.c b/mm.c
> index 64b3292..31aaf83 100644
> --- a/mm.c
> +++ b/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/netfront.c b/netfront.c
> index 42bb103..0eca5b5 100644
> --- a/netfront.c
> +++ b/netfront.c
> @@ -325,8 +325,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/sched.c b/sched.c
> index d0c607e..1e843d9 100644
> --- a/sched.c
> +++ b/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/test.c b/test.c
> index 0d7aba4..154af49 100644
> --- a/test.c
> +++ b/test.c
> @@ -129,7 +129,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);
> @@ -250,7 +250,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/tpm_tis.c b/tpm_tis.c
> index debcc43..98fe837 100644
> --- a/tpm_tis.c
> +++ b/tpm_tis.c
> @@ -912,7 +912,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;
>     }
>  
> @@ -944,7 +944,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);
> @@ -983,7 +983,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;
>     }
>  
> @@ -1145,7 +1145,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/xenbus/xenbus.c b/xenbus/xenbus.c
> index 934f23b..4613ed6 100644
> --- a/xenbus/xenbus.c
> +++ b/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)



_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel