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

[Thomas Leonard <talex5@xxxxxxxxx>]

On 7 August 2014 15:55, Samuel Thibault <samuel.thibault@xxxxxxxxxxxx> 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: 0001-mini-os-enable-compiler-check-for-printk-format-type.patch
Description: Text Data

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