Re: [PATCH] fix printk format compiler warnings

Previous thread: [PATCH] .init.rodata and modpost adjustments by Jan Beulich on Friday, September 5, 2008 - 5:03 am. (1 message)

Next thread: [PATCH] x86: x86_{phys,virt}_bits field also for i386 (v2) by Jan Beulich on Friday, September 5, 2008 - 5:07 am. (15 messages)
From: Jan Beulich
Date: Friday, September 5, 2008 - 5:06 am

%llx and the like cannot be used on u64-derived data types - they must
be cast to long long explicitly for arch-es where u64 is a typedef of
unsigned long (ia64 is where I observed the problem).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 drivers/pci/probe.c     |   16 +++++++++++++---
 drivers/pci/setup-bus.c |    6 +++++-
 2 files changed, 18 insertions(+), 4 deletions(-)

--- linux-2.6.27-rc5/drivers/pci/probe.c	2008-08-21 14:37:32.000000000 +0200
+++ 2.6.27-rc5-pci-probe-print/drivers/pci/probe.c	2008-09-02 15:18:08.000000000 +0200
@@ -383,7 +383,10 @@ void __devinit pci_read_bridge_bases(str
 			res->start = base;
 		if (!res->end)
 			res->end = limit + 0xfff;
-		printk(KERN_INFO "PCI: bridge %s io port: [%llx, %llx]\n", pci_name(dev), res->start, res->end);
+		printk(KERN_INFO "PCI: bridge %s io port: [%Lx, %Lx]\n",
+		       pci_name(dev),
+		       (unsigned long long)res->start,
+		       (unsigned long long)res->end);
 	}
 
 	res = child->resource[1];
@@ -395,7 +398,10 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		res->start = base;
 		res->end = limit + 0xfffff;
-		printk(KERN_INFO "PCI: bridge %s 32bit mmio: [%llx, %llx]\n", pci_name(dev), res->start, res->end);
+		printk(KERN_INFO "PCI: bridge %s 32bit mmio: [%Lx, %Lx]\n",
+		       pci_name(dev),
+		       (unsigned long long)res->start,
+		       (unsigned long long)res->end);
 	}
 
 	res = child->resource[2];
@@ -431,7 +437,11 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
 		res->start = base;
 		res->end = limit + 0xfffff;
-		printk(KERN_INFO "PCI: bridge %s %sbit mmio pref: [%llx, %llx]\n", pci_name(dev), (res->flags & PCI_PREF_RANGE_TYPE_64)?"64":"32",res->start, res->end);
+		printk(KERN_INFO "PCI: bridge %s %dbit mmio pref: [%Lx, %Lx]\n",
+		       pci_name(dev),
+		       res->flags & PCI_PREF_RANGE_TYPE_64 ? ...
From: Andrew Morton
Date: Friday, September 5, 2008 - 2:20 pm

On Fri, 05 Sep 2008 13:06:24 +0100

These have been fixed in linux-next for a month.

Jesse, can we get that into 2.6.27 please?
--

From: Jesse Barnes
Date: Monday, September 8, 2008 - 9:54 pm

I suppose.  They only appear with certain configs and are fairly harmless, so 
I wasn't planning on pushing them into 2.6.27.  But I'll include it in my 
next pull request anyway, since you're such a cool guy and I want to make you 
happy :)  (assuming 2.6.27 proper isn't released in the next couple of 
day...)

Thanks,
Jesse
--

From: Geert Uytterhoeven
Date: Tuesday, September 30, 2008 - 4:29 am

Why did you replace `%ll' by `%L'?

While `L' is used as an internal flag in Linux' vsnprintf() implementation and
is still supported because of historical (pre-C99) reasons, the `L' conversion
qualifier is meant for the `long double' floating point type, as per C99.
The recommended conversion qualifier for the `long long' integer type is `ll'.

Perhaps checkpatch should check for this? Ah, it already does for some of the
integer formats. The patch below adds checks for the missing ones.

Subject: [PATCH] checkpatch: Check for %L for all integer formats

Checkpatch checks for the use of the non-standard `L' conversion qualifier for
some (`u', `d', and `i') of the 6 integers formats.  Add the missing ones
(`o', `x', and `X'), and keep the list sorted.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bc67793..92cbf5e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2277,12 +2277,12 @@ sub process {
 			WARN("usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr);
 		}
 
-# check for %L{u,d,i} in strings
+# check for %L{d,i,o,u,x,X} in strings
 		my $string;
 		while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
 			$string = substr($rawline, $-[1], $+[1] - $-[1]);
-			if ($string =~ /(?<!%)%L[udi]/) {
-				WARN("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr);
+			if ($string =~ /(?<!%)%L[diouxX]/) {
+				WARN("\%L[diouxX] are not-standard C, use %ll[diouxX]\n" . $herecurr);
 				last;
 			}
 		}


With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village 

[...]

Yes that looks entirly reasonable to me.  Will queue it up with my next
batch for Andrew.

Thanks.

-apw
--


I understand all of this, but can't see why you would care about floating point
formats in the kernel (when specifically you know that vsnprintf() & Co aren't
able to handle it anyway).

So I think rather than widening the set of specifiers checkpatch looks for, the
check should rather be removed altogether.

Jan

--

Previous thread: [PATCH] .init.rodata and modpost adjustments by Jan Beulich on Friday, September 5, 2008 - 5:03 am. (1 message)

Next thread: [PATCH] x86: x86_{phys,virt}_bits field also for i386 (v2) by Jan Beulich on Friday, September 5, 2008 - 5:07 am. (15 messages)