For all supported versions of gcc (major version 3 and above), functions
and variables may be declared with __attribute__((unused)) to suppress
warnings if they are declared but unused.
This shouldn't be confused with functions being declared with
__attribute__((used)). This specifies that the function code shall still
be emitted even if it appears to be unreferenced, normally used if
embedded in inline assembly. For gcc 3.4 and later, unreferenced static
variables and functions are not emitted so this attribute is necessary to
force variables and functions to be output. Earlier versions of gcc can
simply use __attribute__((unused)) to suppress warnings about such
variables: we do not require any special classification to ensure they are
emitted.
We introduce __attribute_unused__ for variables that should not produce a
compile warning if they can, due to preprocessor macros, go unreferenced.
Signed-off-by: David Rientjes <[email protected]>
---
include/linux/compiler-gcc.h | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -37,3 +37,19 @@
#define noinline __attribute__((noinline))
#define __attribute_pure__ __attribute__((pure))
#define __attribute_const__ __attribute__((__const__))
+
+/*
+ * __attribute_unused__ shall be used for functions or variables to suppress
+ * warnings when they may be declared but, due to preprocessor macros,
+ * commenting, etc., go unreferenced.
+ *
+ * In contrast, __attribute_used__ shall be used only for functions. gcc <3.4
+ * emits code for static functions that are unreferenced and outputs a warning.
+ * __attribute_used__ will correctly suppress this warning. gcc >=3.4 does not
+ * emit code for static functions that are unreferenced (and thus there is no
+ * warning), but __attribute_used__ forces the function code to be output. Use
+ * __attribute_unused__ to suppress warnings about functions being unused or
+ * __attribute_used__ to ensure code is emitted when it is referenced only in
+ * inline assembly.
+ */
+#define __attribute_unused__ __attribute__((unused))
In the case of !CONFIG_PCI_DIRECT && !CONFIG_PCI_MMCONFIG, type is
unreferened.
Cc: Andi Kleen <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/i386/pci/init.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/i386/pci/init.c b/arch/i386/pci/init.c
--- a/arch/i386/pci/init.c
+++ b/arch/i386/pci/init.c
@@ -6,7 +6,7 @@
in the right sequence from here. */
static __init int pci_access_init(void)
{
- int type = 0;
+ int type __attribute_unused__ = 0;
#ifdef CONFIG_PCI_DIRECT
type = pci_direct_probe();
Replace automatic variable instances of __attribute__((unused)) with
__attribute_unused__ in mca_nmi_hook().
Cc: James Bottomley <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/i386/mach-voyager/voyager_basic.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/i386/mach-voyager/voyager_basic.c b/arch/i386/mach-voyager/voyager_basic.c
--- a/arch/i386/mach-voyager/voyager_basic.c
+++ b/arch/i386/mach-voyager/voyager_basic.c
@@ -292,8 +292,8 @@ machine_emergency_restart(void)
void
mca_nmi_hook(void)
{
- __u8 dumpval __attribute__((unused)) = inb(0xf823);
- __u8 swnmi __attribute__((unused)) = inb(0xf813);
+ __u8 dumpval __attribute_unused__ = inb(0xf823);
+ __u8 swnmi __attribute_unused__ = inb(0xf813);
/* FIXME: assume dump switch pressed */
/* check to see if the dump switch was pressed */
Labeling a variable as __attribute_used__ is ambiguous: it means
__attribute__((unused)) for gcc <3.4 and __attribute__((used)) for
gcc >=3.4. There is no such thing as labeling a variable as
__attribute__((used)). We assume that we're simply suppressing a warning
here if gdthtable[] is declared but unreferenced.
Cc: Achim Leubner <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
drivers/scsi/gdth.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -876,7 +876,7 @@ static int __init gdth_search_pci(gdth_pci_str *pcistr)
/* Vortex only makes RAID controllers.
* We do not really want to specify all 550 ids here, so wildcard match.
*/
-static struct pci_device_id gdthtable[] __attribute_used__ = {
+static struct pci_device_id gdthtable[] __attribute_unused__ = {
{PCI_VENDOR_ID_VORTEX,PCI_ANY_ID,PCI_ANY_ID, PCI_ANY_ID},
{PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC,PCI_ANY_ID,PCI_ANY_ID},
{PCI_VENDOR_ID_INTEL,PCI_DEVICE_ID_INTEL_SRC_XSCALE,PCI_ANY_ID,PCI_ANY_ID},
Replace function instances of __attribute__ ((unused)) with
__attribute_unused__.
Cc: Geoff Levand <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/powerpc/platforms/ps3/interrupt.c | 4 ++--
arch/powerpc/platforms/ps3/time.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -434,7 +434,7 @@ static void _dump_64_bmp(const char *header, const u64 *p, unsigned cpu,
*p & 0xffff);
}
-static void __attribute__ ((unused)) _dump_256_bmp(const char *header,
+static void __attribute_unused__ _dump_256_bmp(const char *header,
const u64 *p, unsigned cpu, const char* func, int line)
{
pr_debug("%s:%d: %s %u {%016lx:%016lx:%016lx:%016lx}\n",
@@ -453,7 +453,7 @@ static void _dump_bmp(struct ps3_private* pd, const char* func, int line)
}
#define dump_mask(_x) _dump_mask(_x, __func__, __LINE__)
-static void __attribute__ ((unused)) _dump_mask(struct ps3_private* pd,
+static void __attribute_unused__ _dump_mask(struct ps3_private* pd,
const char* func, int line)
{
unsigned long flags;
diff --git a/arch/powerpc/platforms/ps3/time.c b/arch/powerpc/platforms/ps3/time.c
--- a/arch/powerpc/platforms/ps3/time.c
+++ b/arch/powerpc/platforms/ps3/time.c
@@ -39,7 +39,7 @@ static void _dump_tm(const struct rtc_time *tm, const char* func, int line)
}
#define dump_time(_a) _dump_time(_a, __func__, __LINE__)
-static void __attribute__ ((unused)) _dump_time(int time, const char* func,
+static void __attribute_unused__ _dump_time(int time, const char* func,
int line)
{
struct rtc_time tm;
Replace automatic variable instances of __attribute__ ((unused)) with
__attribute_unused__.
Cc: Andy Whitcroft <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
include/asm-i386/mmzone.h | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/asm-i386/mmzone.h b/include/asm-i386/mmzone.h
--- a/include/asm-i386/mmzone.h
+++ b/include/asm-i386/mmzone.h
@@ -122,21 +122,21 @@ static inline int pfn_valid(int pfn)
__alloc_bootmem_node(NODE_DATA(0), (x), PAGE_SIZE, 0)
#define alloc_bootmem_node(pgdat, x) \
({ \
- struct pglist_data __attribute__ ((unused)) \
+ struct pglist_data __attribute_unused__ \
*__alloc_bootmem_node__pgdat = (pgdat); \
__alloc_bootmem_node(NODE_DATA(0), (x), SMP_CACHE_BYTES, \
__pa(MAX_DMA_ADDRESS)); \
})
#define alloc_bootmem_pages_node(pgdat, x) \
({ \
- struct pglist_data __attribute__ ((unused)) \
+ struct pglist_data __attribute_unused__ \
*__alloc_bootmem_node__pgdat = (pgdat); \
__alloc_bootmem_node(NODE_DATA(0), (x), PAGE_SIZE, \
__pa(MAX_DMA_ADDRESS)) \
})
#define alloc_bootmem_low_pages_node(pgdat, x) \
({ \
- struct pglist_data __attribute__ ((unused)) \
+ struct pglist_data __attribute_unused__ \
*__alloc_bootmem_node__pgdat = (pgdat); \
__alloc_bootmem_node(NODE_DATA(0), (x), PAGE_SIZE, 0); \
})
Replace function instances of __attribute__((unused)) with
__attribute_unused__ to suppress warnings.
Cc: David Howells <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/frv/kernel/gdb-stub.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/frv/kernel/gdb-stub.c b/arch/frv/kernel/gdb-stub.c
--- a/arch/frv/kernel/gdb-stub.c
+++ b/arch/frv/kernel/gdb-stub.c
@@ -1195,7 +1195,7 @@ static void gdbstub_check_breakpoint(void)
/*
*
*/
-static void __attribute__((unused)) gdbstub_show_regs(void)
+static void __attribute_unused__ gdbstub_show_regs(void)
{
unsigned long *reg;
int loop;
@@ -1223,7 +1223,7 @@ static void __attribute__((unused)) gdbstub_show_regs(void)
/*
* dump debugging regs
*/
-static void __attribute__((unused)) gdbstub_dump_debugregs(void)
+static void __attribute_unused__ gdbstub_dump_debugregs(void)
{
gdbstub_printk("DCR %08lx ", __debug_status.dcr);
gdbstub_printk("BRR %08lx\n", __debug_status.brr);
@@ -2079,25 +2079,25 @@ void gdbstub_exit(int status)
* GDB wants to call malloc() and free() to allocate memory for calling kernel
* functions directly from its command line
*/
-static void *malloc(size_t size) __attribute__((unused));
+static void *malloc(size_t size) __attribute_unused__;
static void *malloc(size_t size)
{
return kmalloc(size, GFP_ATOMIC);
}
-static void free(void *p) __attribute__((unused));
+static void free(void *p) __attribute_unused__;
static void free(void *p)
{
kfree(p);
}
-static uint32_t ___get_HSR0(void) __attribute__((unused));
+static uint32_t ___get_HSR0(void) __attribute_unused__;
static uint32_t ___get_HSR0(void)
{
return __get_HSR(0);
}
-static uint32_t ___set_HSR0(uint32_t x) __attribute__((unused));
+static uint32_t ___set_HSR0(uint32_t x) __attribute_unused__;
static uint32_t ___set_HSR0(uint32_t x)
{
__set_HSR(0, x);
Replace function instances of __attribute__((unused)) with
__attribute_unused__.
Cc: Ralf Baechle <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/mips/mm/tlbex.c | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -35,24 +35,24 @@
#include <asm/smp.h>
#include <asm/war.h>
-static __init int __attribute__((unused)) r45k_bvahwbug(void)
+static __init int __attribute_unused__ r45k_bvahwbug(void)
{
/* XXX: We should probe for the presence of this bug, but we don't. */
return 0;
}
-static __init int __attribute__((unused)) r4k_250MHZhwbug(void)
+static __init int __attribute_unused__ r4k_250MHZhwbug(void)
{
/* XXX: We should probe for the presence of this bug, but we don't. */
return 0;
}
-static __init int __attribute__((unused)) bcm1250_m3_war(void)
+static __init int __attribute_unused__ bcm1250_m3_war(void)
{
return BCM1250_M3_WAR;
}
-static __init int __attribute__((unused)) r10000_llsc_war(void)
+static __init int __attribute_unused__ r10000_llsc_war(void)
{
return R10000_LLSC_WAR;
}
@@ -511,18 +511,18 @@ L_LA(_r3000_write_probe_fail)
#define i_ehb(buf) i_sll(buf, 0, 0, 3)
#ifdef CONFIG_64BIT
-static __init int __attribute__((unused)) in_compat_space_p(long addr)
+static __init int __attribute_unused__ in_compat_space_p(long addr)
{
/* Is this address in 32bit compat space? */
return (((addr) & 0xffffffff00000000L) == 0xffffffff00000000L);
}
-static __init int __attribute__((unused)) rel_highest(long val)
+static __init int __attribute_unused__ rel_highest(long val)
{
return ((((val + 0x800080008000L) >> 48) & 0xffff) ^ 0x8000) - 0x8000;
}
-static __init int __attribute__((unused)) rel_higher(long val)
+static __init int __attribute_unused__ rel_higher(long val)
{
return ((((val + 0x80008000L) >> 32) & 0xffff) ^ 0x8000) - 0x8000;
}
@@ -556,8 +556,8 @@ static __init void i_LA_mostly(u32 **buf, unsigned int rs, long addr)
i_lui(buf, rs, rel_hi(addr));
}
-static __init void __attribute__((unused)) i_LA(u32 **buf, unsigned int rs,
- long addr)
+static __init void __attribute_unused__ i_LA(u32 **buf, unsigned int rs,
+ long addr)
{
i_LA_mostly(buf, rs, addr);
if (rel_lo(addr))
@@ -636,8 +636,8 @@ static __init void copy_handler(struct reloc *rel, struct label *lab,
move_labels(lab, first, end, off);
}
-static __init int __attribute__((unused)) insn_has_bdelay(struct reloc *rel,
- u32 *addr)
+static __init int __attribute_unused__ insn_has_bdelay(struct reloc *rel,
+ u32 *addr)
{
for (; rel->lab != label_invalid; rel++) {
if (rel->addr == addr
@@ -650,15 +650,15 @@ static __init int __attribute__((unused)) insn_has_bdelay(struct reloc *rel,
}
/* convenience functions for labeled branches */
-static void __init __attribute__((unused))
+static void __init __attribute_unused__
il_bltz(u32 **p, struct reloc **r, unsigned int reg, enum label_id l)
{
r_mips_pc16(r, *p, l);
i_bltz(p, reg, 0);
}
-static void __init __attribute__((unused)) il_b(u32 **p, struct reloc **r,
- enum label_id l)
+static void __init __attribute_unused__ il_b(u32 **p, struct reloc **r,
+ enum label_id l)
{
r_mips_pc16(r, *p, l);
i_b(p, 0);
@@ -671,7 +671,7 @@ static void __init il_beqz(u32 **p, struct reloc **r, unsigned int reg,
i_beqz(p, reg, 0);
}
-static void __init __attribute__((unused))
+static void __init __attribute_unused__
il_beqzl(u32 **p, struct reloc **r, unsigned int reg, enum label_id l)
{
r_mips_pc16(r, *p, l);
@@ -692,7 +692,7 @@ static void __init il_bgezl(u32 **p, struct reloc **r, unsigned int reg,
i_bgezl(p, reg, 0);
}
-static void __init __attribute__((unused))
+static void __init __attribute_unused__
il_bgez(u32 **p, struct reloc **r, unsigned int reg, enum label_id l)
{
r_mips_pc16(r, *p, l);
@@ -810,7 +810,7 @@ static __initdata u32 final_handler[64];
*
* As if we MIPS hackers wouldn't know how to nop pipelines happy ...
*/
-static __init void __attribute__((unused)) build_tlb_probe_entry(u32 **p)
+static __init void __attribute_unused__ build_tlb_probe_entry(u32 **p)
{
switch (current_cpu_data.cputype) {
/* Found by experiment: R4600 v2.0 needs this, too. */
@@ -1098,7 +1098,7 @@ build_get_pgd_vmalloc64(u32 **p, struct label **l, struct reloc **r,
* TMP and PTR are scratch.
* TMP will be clobbered, PTR will hold the pgd entry.
*/
-static __init void __attribute__((unused))
+static __init void __attribute_unused__
build_get_pgde32(u32 **p, unsigned int tmp, unsigned int ptr)
{
long pgdc = (long)pgd_current;
Replace variable instances of __attribute__((unused)) with
__attribute_unused__.
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Koeller <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
arch/mips/basler/excite/excite_device.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/mips/basler/excite/excite_device.c b/arch/mips/basler/excite/excite_device.c
--- a/arch/mips/basler/excite/excite_device.c
+++ b/arch/mips/basler/excite/excite_device.c
@@ -68,7 +68,7 @@ enum {
static struct resource
- excite_ctr_resource __attribute__((unused)) = {
+ excite_ctr_resource __attribute_unused__ = {
.name = "GPI counters",
.start = 0,
.end = 5,
@@ -77,7 +77,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_gpislice_resource __attribute__((unused)) = {
+ excite_gpislice_resource __attribute_unused__ = {
.name = "GPI slices",
.start = 0,
.end = 1,
@@ -86,7 +86,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_mdio_channel_resource __attribute__((unused)) = {
+ excite_mdio_channel_resource __attribute_unused__ = {
.name = "MDIO channels",
.start = 0,
.end = 1,
@@ -95,7 +95,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_fifomem_resource __attribute__((unused)) = {
+ excite_fifomem_resource __attribute_unused__ = {
.name = "FIFO memory",
.start = 0,
.end = 767,
@@ -104,7 +104,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_scram_resource __attribute__((unused)) = {
+ excite_scram_resource __attribute_unused__ = {
.name = "Scratch RAM",
.start = EXCITE_PHYS_SCRAM,
.end = EXCITE_PHYS_SCRAM + EXCITE_SIZE_SCRAM - 1,
@@ -113,7 +113,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_fpga_resource __attribute__((unused)) = {
+ excite_fpga_resource __attribute_unused__ = {
.name = "System FPGA",
.start = EXCITE_PHYS_FPGA,
.end = EXCITE_PHYS_FPGA + EXCITE_SIZE_FPGA - 1,
@@ -122,7 +122,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_nand_resource __attribute__((unused)) = {
+ excite_nand_resource __attribute_unused__ = {
.name = "NAND flash control",
.start = EXCITE_PHYS_NAND,
.end = EXCITE_PHYS_NAND + EXCITE_SIZE_NAND - 1,
@@ -131,7 +131,7 @@ static struct resource
.sibling = NULL,
.child = NULL
},
- excite_titan_resource __attribute__((unused)) = {
+ excite_titan_resource __attribute_unused__ = {
.name = "TITAN registers",
.start = EXCITE_PHYS_TITAN,
.end = EXCITE_PHYS_TITAN + EXCITE_SIZE_TITAN - 1,
There is no such thing as labeling a variable as __attribute__((used)).
Since ts_shift is not referenced in inline assembly, we assume that we're
simply suppressing a warning here if the variable is declared but
unreferenced.
Cc: Paul Mundt <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
---
include/asm-sh/cpu-sh3/dma.h | 2 +-
include/asm-sh/cpu-sh4/dma-sh7780.h | 2 +-
include/asm-sh/cpu-sh4/dma.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/asm-sh/cpu-sh3/dma.h b/include/asm-sh/cpu-sh3/dma.h
--- a/include/asm-sh/cpu-sh3/dma.h
+++ b/include/asm-sh/cpu-sh3/dma.h
@@ -26,7 +26,7 @@ enum {
XMIT_SZ_128BIT,
};
-static unsigned int ts_shift[] __attribute__ ((used)) = {
+static unsigned int ts_shift[] __attribute_unused__ = {
[XMIT_SZ_8BIT] = 0,
[XMIT_SZ_16BIT] = 1,
[XMIT_SZ_32BIT] = 2,
diff --git a/include/asm-sh/cpu-sh4/dma-sh7780.h b/include/asm-sh/cpu-sh4/dma-sh7780.h
--- a/include/asm-sh/cpu-sh4/dma-sh7780.h
+++ b/include/asm-sh/cpu-sh4/dma-sh7780.h
@@ -28,7 +28,7 @@ enum {
/*
* The DMA count is defined as the number of bytes to transfer.
*/
-static unsigned int __attribute__ ((used)) ts_shift[] = {
+static unsigned int ts_shift[] __attribute_unused__ = {
[XMIT_SZ_8BIT] = 0,
[XMIT_SZ_16BIT] = 1,
[XMIT_SZ_32BIT] = 2,
diff --git a/include/asm-sh/cpu-sh4/dma.h b/include/asm-sh/cpu-sh4/dma.h
--- a/include/asm-sh/cpu-sh4/dma.h
+++ b/include/asm-sh/cpu-sh4/dma.h
@@ -53,7 +53,7 @@ enum {
/*
* The DMA count is defined as the number of bytes to transfer.
*/
-static unsigned int ts_shift[] __attribute__ ((used)) = {
+static unsigned int ts_shift[] __attribute_unused__ = {
[XMIT_SZ_64BIT] = 3,
[XMIT_SZ_8BIT] = 0,
[XMIT_SZ_16BIT] = 1,
On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
> +#define __attribute_unused__ __attribute__((unused))
Suggest __unused which is shorter and looks compiler-neutral.
On Tue, 2007-05-01 at 21:28 -0700, David Rientjes wrote:
> For all supported versions of gcc (major version 3 and above), functions
> and variables may be declared with __attribute__((unused)) to suppress
> warnings if they are declared but unused.
Adding this macro doesn't give us anything that simply saying
"__attribute__((unused))" doesn't give. But it does add a layer of
kernel-specific indirection.
If we're going to get kernel-specific, I'd prefer to see:
__needed: suppress warning and don't discard,
__unneeded: suppress warning and might discard.
For me this fits better with how I think.
Rusty.
On Wed, 2 May 2007, Alexey Dobriyan wrote:
> On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
> > +#define __attribute_unused__ __attribute__((unused))
>
> Suggest __unused which is shorter and looks compiler-neutral.
>
So you would also suggest renaming __attribute_used__ and all 48 of its
uses to __used?
On Wed, 2 May 2007, Rusty Russell wrote:
> Adding this macro doesn't give us anything that simply saying
> "__attribute__((unused))" doesn't give. But it does add a layer of
> kernel-specific indirection.
>
That's obviously true since we're defining __attribute_unused__ to be
__attribute__((unused)).
We were trying to clean up the misconception that the current
__attribute_used__ was created to suppress warnings when, in fact, that
was not its purpose. It was created to emit the code for a function that
appeared to be unreferenced and only suppressed warnings as a side-effect
in gcc <3.4.
> If we're going to get kernel-specific, I'd prefer to see:
>
> __needed: suppress warning and don't discard,
That would be the current definition of __attribute_used__ (i.e. we're
saying that we use the function in inline assembly even though it appears
we don't use it at all).
> __unneeded: suppress warning and might discard.
>
That would be the patched definition of __attribute_unused__.
So let's go back to the problem this was initially supposed to fix from
arch/i386/pci/init.c:
static __init int pci_access_init(void)
{
int type = 0;
#ifdef CONFIG_PCI_DIRECT
type = pci_direct_probe();
#endif
#ifdef CONFIG_PCI_MMCONFIG
pci_mmcfg_init(type);
#endif
...
and type is unreferenced for the remainder of the function. Obviously we
could add #if defined(CONFIG_PCI_DIRECT) || defined(CONFIG_PCI_MMCONFIG)
before the declaration of 'type', but that becomes sloppy pretty quickly.
The patched version makes this:
int type __attribute_unused__ = 0;
which definitely tells you that you're using a compiler attribute that
will be attached to that automatic. In your case:
int type __unneeded = 0;
doesn't say anything in this case. It doesn't resemble any attribute that
a programmer might be familiar with and begs the question of why we've
declared it if it's truly "unneeded"?
By the way, there are tons of these instances where __attribute__((used))
needs to be added in driver code to suppress unreferenced warnings.
David
On Tue, 1 May 2007 22:53:52 -0700 (PDT) David Rientjes <[email protected]> wrote:
> On Wed, 2 May 2007, Alexey Dobriyan wrote:
>
> > On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
> > > +#define __attribute_unused__ __attribute__((unused))
> >
> > Suggest __unused which is shorter and looks compiler-neutral.
> >
>
> So you would also suggest renaming __attribute_used__ and all 48 of its
> uses to __used?
Or __needed or __unneeded. None of them mean much to me and I'd be forever
going back to the definition to work out what was intended.
We're still in search of a name, IMO. But once we have it, yeah, we should
update all present users. We can do that over time: retain the old and new
definitions for a while.
On Tue, 1 May 2007, David Rientjes wrote:
> The patched version makes this:
>
> int type __attribute_unused__ = 0;
>
> which definitely tells you that you're using a compiler attribute that
> will be attached to that automatic. In your case:
>
> int type __unneeded = 0;
>
> doesn't say anything in this case. It doesn't resemble any attribute that
> a programmer might be familiar with and begs the question of why we've
> declared it if it's truly "unneeded"?
>
One possible way to remedy this situation is with __needed and
__maybe_unneeded.
__needed would be defined to __attribute__ ((used)), which would apply to
functions only and specify that its code needs to be emitted anyway even
though it appears to be unreferenced. This is needed for gcc >=3.4. In
gcc <3.4, this gets defined to be __attribute__ ((unused)) to simply
suppress the warning. So now all functions that are unreferenced except
in inline assembly get __needed appended.
__maybe_unneeded would be defined to __attribute__ ((unused)). It can
apply to either functions or variables to suppress the warning if they are
unused.
On Tue, May 01, 2007 at 10:53:52PM -0700, David Rientjes wrote:
>On Wed, 2 May 2007, Alexey Dobriyan wrote:
>
>> On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
>> > +#define __attribute_unused__ __attribute__((unused))
>>
>> Suggest __unused which is shorter and looks compiler-neutral.
>>
>
>So you would also suggest renaming __attribute_used__ and all 48 of its
>uses to __used?
I suggest. ;-p
'__attribute_unused__' is really long. I would prefer '__attribute__((unused))', since your macro doesn't let me type much less characters.
On Tue, 2007-05-01 at 23:06 -0700, David Rientjes wrote:
> On Wed, 2 May 2007, Rusty Russell wrote:
>
> > Adding this macro doesn't give us anything that simply saying
> > "__attribute__((unused))" doesn't give. But it does add a layer of
> > kernel-specific indirection.
> >
>
> That's obviously true since we're defining __attribute_unused__ to be
> __attribute__((unused)).
Hi David,
I'm horribly familiar with this issue, BTW, so we don't need so many
words 8)
> The patched version makes this:
>
> int type __attribute_unused__ = 0;
>
> which definitely tells you that you're using a compiler attribute that
> will be attached to that automatic. In your case:
>
> int type __unneeded = 0;
>
> doesn't say anything in this case. It doesn't resemble any attribute that
> a programmer might be familiar with and begs the question of why we've
> declared it if it's truly "unneeded"?
Your version makes one wonder why they didn't use
"__attribute__((unused))". Obviously the __attribute_unused__ macro
exists for a reason, so they wonder what's the difference between that
and the attribute? The answer: nothing.
OTOH, your point about "__unneeded" is well taken. "__needed" and
"__optional" perhaps? But their feature is *exactly* that the don't
look like the gcc attributes, hence avoid their semantic screwage.
> By the way, there are tons of these instances where __attribute__((used))
> needs to be added in driver code to suppress unreferenced warnings.
Sure; historically we refactor around it. But warnings are now so
commonplace few people care 8(
Cheers,
Rusty.
On Wed, 2 May 2007, Rusty Russell wrote:
> OTOH, your point about "__unneeded" is well taken. "__needed" and
> "__optional" perhaps? But their feature is *exactly* that the don't
> look like the gcc attributes, hence avoid their semantic screwage.
>
Hmm, __optional doesn't sound appropriate either. Since this is going to
be defined to be __attribute__ ((unused)), it can apply to both functions
and variables. It should be applied to a function if it truly is
unreferenced within the tree (and there are several examples of this
current HEAD) and we don't want to use __needed because it still emits the
function code even though it suppresses the warning. So saying a function
that has no callers is "__optional" makes no sense since its code isn't
going to be emitted in gcc >=3.4.
What's your opinion of my __needed and __maybe_unused idea such as the
following?
compiler: define __maybe_unused
Define __maybe_unused to apply to both functions or variables as
__attribute__((unused)). This will not emit a compile-time warning when
a function or variable is declared but unreferenced.
We eventually want to change the name of __attribute_used__ to __used.
Signed-off-by: David Rientjes <[email protected]>
---
include/linux/compiler-gcc.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -37,3 +37,4 @@
#define noinline __attribute__((noinline))
#define __attribute_pure__ __attribute__((pure))
#define __attribute_const__ __attribute__((__const__))
+#define __maybe_unused __attribute__((unused))
Andrew Morton wrote:
> On Tue, 1 May 2007 22:53:52 -0700 (PDT) David Rientjes <[email protected]> wrote:
>
>
>>On Wed, 2 May 2007, Alexey Dobriyan wrote:
>>
>>
>>>On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
>>>
>>>>+#define __attribute_unused__ __attribute__((unused))
>>>
>>>Suggest __unused which is shorter and looks compiler-neutral.
>>>
>>
>>So you would also suggest renaming __attribute_used__ and all 48 of its
>>uses to __used?
>
>
> Or __needed or __unneeded. None of them mean much to me and I'd be forever
> going back to the definition to work out what was intended.
>
> We're still in search of a name, IMO. But once we have it, yeah, we should
> update all present users. We can do that over time: retain the old and new
> definitions for a while.
maybe_unused?
The used attribute IMO is a bit easier to parse, so I don't think that
needs to be renamed.
Regarding the used vs needed thing, I don't think needed adds very much
and deviates from gcc terminology. Presumably if something is used it is
needed, and vice versa; similarly for unused.
--
SUSE Labs, Novell Inc.
On Tue, 1 May 2007 23:41:34 -0700 (PDT) David Rientjes <[email protected]> wrote:
> compiler: define __maybe_unused
>
> Define __maybe_unused to apply to both functions or variables as
> __attribute__((unused)). This will not emit a compile-time warning when
> a function or variable is declared but unreferenced.
>
> We eventually want to change the name of __attribute_used__ to __used.
>
> Signed-off-by: David Rientjes <[email protected]>
> ---
> include/linux/compiler-gcc.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -37,3 +37,4 @@
> #define noinline __attribute__((noinline))
> #define __attribute_pure__ __attribute__((pure))
> #define __attribute_const__ __attribute__((__const__))
> +#define __maybe_unused __attribute__((unused))
Seems sane to me. We'd need a definition in compiler-intel.h too. I don't
know if ICC implements __attribute__((unused)) - probably it does.
I guess we can get by without any commentary describing __maybe_unused, but
I think __used would need one - it's pretty obscure.
+@cindex @code{used} attribute.
+@item used
+This attribute, attached to a function, means that code must be emitted
+for the function even if it appears that the function is not referenced.
+This is useful, for example, when the function is referenced only in
+inline assembly.
On Wed, 2 May 2007, Alexey Dobriyan wrote:
> On Tue, May 01, 2007 at 09:28:18PM -0700, David Rientjes wrote:
> > +#define __attribute_unused__ __attribute__((unused))
>
> Suggest __unused which is shorter and looks compiler-neutral.
not only that, but there are numerous files that *already* use
"__unused":
$ grep -rw __unused *
... snip lots of output here ...
as well as a few files that can now have their definition of that
removed:
$ grep -r "define __unused" *
drivers/net/defxx.c:#define __unused __attribute__ ((unused))
drivers/net/declance.c:#define __unused __attribute__ ((unused))
drivers/misc/thinkpad_acpi.c:#define __unused __attribute__ ((unused))
i think "__unused" is the clear choice here.
rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
On Tue, 2007-05-01 at 23:41 -0700, David Rientjes wrote:
> On Wed, 2 May 2007, Rusty Russell wrote:
>
> > OTOH, your point about "__unneeded" is well taken. "__needed" and
> > "__optional" perhaps? But their feature is *exactly* that the don't
> > look like the gcc attributes, hence avoid their semantic screwage.
> >
>
> Hmm, __optional doesn't sound appropriate either. Since this is going to
> be defined to be __attribute__ ((unused)), it can apply to both functions
> and variables. It should be applied to a function if it truly is
> unreferenced within the tree (and there are several examples of this
> current HEAD) and we don't want to use __needed because it still emits the
> function code even though it suppresses the warning. So saying a function
> that has no callers is "__optional" makes no sense since its code isn't
> going to be emitted in gcc >=3.4.
Hi David,
That sounds exactly right to me! If the author says it's optional, it
might be discarded. If they say it's needed, it won't be. At least,
when I'm coding and gcc warns me something is unused, this is the
decision I have to make ("is this really needed or not?").
> What's your opinion of my __needed and __maybe_unused idea such as the
> following?
You mean __used and __maybe_unused? I still think there's a gap between
what the author wants to say ("No, you really have to keep this!" or
"OK, this might not be necessary") and the concept of "used" and
"unused".
Cheers,
Rusty.
On Wed, 2 May 2007, Robert P. J. Day wrote:
> not only that, but there are numerous files that *already* use
> "__unused":
>
> $ grep -rw __unused *
> ... snip lots of output here ...
>
> as well as a few files that can now have their definition of that
> removed:
>
> $ grep -r "define __unused" *
> drivers/net/defxx.c:#define __unused __attribute__ ((unused))
> drivers/net/declance.c:#define __unused __attribute__ ((unused))
> drivers/misc/thinkpad_acpi.c:#define __unused __attribute__ ((unused))
>
> i think "__unused" is the clear choice here.
>
No, it's not the clear choice. This would apply to both functions and
variables so the suppress a compiler warning for a variable whose use
depends on preprocessor macros, I would have to use __unused even though
it may be used.
Hence, I recommend __maybe_unused.
On Wed, 2 May 2007, Rusty Russell wrote:
> That sounds exactly right to me! If the author says it's optional, it
> might be discarded. If they say it's needed, it won't be. At least,
> when I'm coding and gcc warns me something is unused, this is the
> decision I have to make ("is this really needed or not?").
>
Hi Rusty,
There are many instances in the tree of functions that have no callers
whatsoever because they've been commented out temporarily, disabled
through configuration, etc. These are marked __attribute__ ((unused))
right now so that the compiler doesn't emit a warning (and with gcc >=3.4
it doesn't even emit code for them). What's __optional about these
functions if they have no callers? They're unused. So we cover all our
bases with __maybe_unused.
On Wed, 2 May 2007, David Rientjes wrote:
> On Wed, 2 May 2007, Robert P. J. Day wrote:
>
> > not only that, but there are numerous files that *already* use
> > "__unused":
> >
> > $ grep -rw __unused *
> > ... snip lots of output here ...
> >
> > as well as a few files that can now have their definition of that
> > removed:
> >
> > $ grep -r "define __unused" *
> > drivers/net/defxx.c:#define __unused __attribute__ ((unused))
> > drivers/net/declance.c:#define __unused __attribute__ ((unused))
> > drivers/misc/thinkpad_acpi.c:#define __unused __attribute__ ((unused))
> >
> > i think "__unused" is the clear choice here.
> >
>
> No, it's not the clear choice. This would apply to both functions
> and variables so the suppress a compiler warning for a variable
> whose use depends on preprocessor macros, I would have to use
> __unused even though it may be used.
>
> Hence, I recommend __maybe_unused.
sorry, i worded that badly. i suggested that "__unused" was the clear
choice only because it already had significant historical precedent
and people on this list are notorious for not liking major, disruptive
changes.
if no one objects to changing the existing occurrences, then no
problem.
rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
On Wed, 2007-05-02 at 00:22 -0700, David Rientjes wrote:
> On Wed, 2 May 2007, Rusty Russell wrote:
>
> > That sounds exactly right to me! If the author says it's optional, it
> > might be discarded. If they say it's needed, it won't be. At least,
> > when I'm coding and gcc warns me something is unused, this is the
> > decision I have to make ("is this really needed or not?").
> >
>
> Hi Rusty,
>
> There are many instances in the tree of functions that have no callers
> whatsoever because they've been commented out temporarily, disabled
> through configuration, etc. These are marked __attribute__ ((unused))
> right now so that the compiler doesn't emit a warning (and with gcc >=3.4
> it doesn't even emit code for them). What's __optional about these
> functions if they have no callers? They're unused. So we cover all our
> bases with __maybe_unused.
Hi David,
If they're really unused, they should be deleted, not
warning-suppressed. The interesting case is where they may or may not
be used because of config options. ie. they're optional.
__maybe_unused does not, at a glance, tell me that it's OK for gcc to
drop them. __optional comes closer. However, it's better than
__unused, so I'll stop now 8)
Thanks,
Rusty.
On Wednesday 02 May 2007 06:28:22 David Rientjes wrote:
> In the case of !CONFIG_PCI_DIRECT && !CONFIG_PCI_MMCONFIG, type is
> unreferened.
The patch didn't compile on i386 defconfig. Fixed now but please
compile test future patches.
-Andi
On Wed, May 02, 2007 at 12:22:24AM -0700, David Rientjes wrote:
> On Wed, 2 May 2007, Rusty Russell wrote:
>
> > That sounds exactly right to me! If the author says it's optional, it
> > might be discarded. If they say it's needed, it won't be. At least,
> > when I'm coding and gcc warns me something is unused, this is the
> > decision I have to make ("is this really needed or not?").
> >
>
> Hi Rusty,
>
> There are many instances in the tree of functions that have no callers
> whatsoever because they've been commented out temporarily, disabled
> through configuration, etc. These are marked __attribute__ ((unused))
> right now so that the compiler doesn't emit a warning (and with gcc >=3.4
> it doesn't even emit code for them). What's __optional about these
> functions if they have no callers? They're unused. So we cover all our
> bases with __maybe_unused.
"many ... are marked __attribute__ ((unused))" is not true:
$ grep -r __attribute_used__ * | wc -l
60
$
static inline functions don't result in warnings.
And for global functions, it is technically impossible for gcc to figure
out whether a function has any users.
Unused static non-inline functions are the only functions resulting in
warnings when being unused.
If we don't want gcc to emit warnings for such, we could disable them
globally.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Wed, May 02, 2007 at 04:55:50PM +0200, Adrian Bunk wrote:
>
> "many ... are marked __attribute__ ((unused))" is not true:
> $ grep -r __attribute_used__ * | wc -l
> 60
> $
Sorry, my fault - I confused used and unused.
>...
> Unused static non-inline functions are the only functions resulting in
> warnings when being unused.
> If we don't want gcc to emit warnings for such, we could disable them
> globally.
But this point still stands:
If we don't want any warnings with CONFIG_PCI=n, CONFIG_SYSFS=n or
CONFIG_PROC_FS=n, we'd have to annotate _many_ functions.
If the lonterm goal is to compile the kernel with -Werror then we need
-Wno-unused-function, not annotating individual functions.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
David Rientjes wrote:
> Replace function instances of __attribute__ ((unused)) with
> __attribute_unused__.
>
> Cc: Geoff Levand <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
> ---
> arch/powerpc/platforms/ps3/interrupt.c | 4 ++--
> arch/powerpc/platforms/ps3/time.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Please give a better comment that explains why this change is
desirable.
-Geoff
On Wed, 2 May 2007, Andi Kleen wrote:
> On Wednesday 02 May 2007 06:28:22 David Rientjes wrote:
> > In the case of !CONFIG_PCI_DIRECT && !CONFIG_PCI_MMCONFIG, type is
> > unreferened.
>
> The patch didn't compile on i386 defconfig. Fixed now but please
> compile test future patches.
>
That's because it's dependent on the first patch in the series.
If it was fixed by replacing __attribute_unused__ with
__attribute__((unused)), then that's fine. __attribute_used__ would be
wrong, however.
On Wed, 2 May 2007, Adrian Bunk wrote:
> If we don't want any warnings with CONFIG_PCI=n, CONFIG_SYSFS=n or
> CONFIG_PROC_FS=n, we'd have to annotate _many_ functions.
>
> If the lonterm goal is to compile the kernel with -Werror then we need
> -Wno-unused-function, not annotating individual functions.
>
That's only addressing part of the issue. What about automatic or static
external variables that are declared but may go unreferenced depending on
preprocessor macros? You need to annotate them with __attribute__
((unused)) to suppress compiler warnings. Globally disabling such
warnings would eventually cause unused code to go unnoticed.
David
On Tue, May 01, 2007 at 09:28:33PM -0700, David Rientjes wrote:
> Replace variable instances of __attribute__((unused)) with
> __attribute_unused__.
>
> Cc: Ralf Baechle <[email protected]>
> Cc: Thomas Koeller <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
Acked-by: Ralf Baechle <[email protected]>
Ralf
On Tue, May 01, 2007 at 09:28:45PM -0700, David Rientjes wrote:
> Replace function instances of __attribute__((unused)) with
> __attribute_unused__.
>
> Cc: Ralf Baechle <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
Acked-by: Ralf Baechle <[email protected]>
Ralf
On Wed, May 02, 2007 at 10:16:15AM -0700, David Rientjes wrote:
> On Wed, 2 May 2007, Adrian Bunk wrote:
>
> > If we don't want any warnings with CONFIG_PCI=n, CONFIG_SYSFS=n or
> > CONFIG_PROC_FS=n, we'd have to annotate _many_ functions.
> >
> > If the lonterm goal is to compile the kernel with -Werror then we need
> > -Wno-unused-function, not annotating individual functions.
>
> That's only addressing part of the issue. What about automatic or static
> external variables that are declared but may go unreferenced depending on
This is only about static code. For non-static code it would be
impossible for gcc to issue warnings.
> preprocessor macros? You need to annotate them with __attribute__
> ((unused)) to suppress compiler warnings. Globally disabling such
> warnings would eventually cause unused code to go unnoticed.
But looking at a kernel build it seems there are far few warnings than I
remembered, so it might actually be possible to annotate all code
accordingly.
> David
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Thu, 3 May 2007, Adrian Bunk wrote:
> > That's only addressing part of the issue. What about automatic or static
> > external variables that are declared but may go unreferenced depending on
>
> This is only about static code. For non-static code it would be
> impossible for gcc to issue warnings.
>
static external variables are certainly still static code and gcc issues
the proper warnings if I do:
static struct bootnode nodes[MAX_NUMNODES];
and I never reference nodes.
On Thu, May 03, 2007 at 11:56:34AM -0700, David Rientjes wrote:
> On Thu, 3 May 2007, Adrian Bunk wrote:
>
> > > That's only addressing part of the issue. What about automatic or static
> > > external variables that are declared but may go unreferenced depending on
> >
> > This is only about static code. For non-static code it would be
> > impossible for gcc to issue warnings.
>
> static external variables are certainly still static code and gcc issues
> the proper warnings if I do:
>
> static struct bootnode nodes[MAX_NUMNODES];
>
> and I never reference nodes.
No disagreement.
But you said:
What about automatic or static external variables that are declared but ...
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Thu, 3 May 2007, Adrian Bunk wrote:
> > static external variables are certainly still static code and gcc issues
> > the proper warnings if I do:
> >
> > static struct bootnode nodes[MAX_NUMNODES];
> >
> > and I never reference nodes.
>
> No disagreement.
>
> But you said:
> What about automatic or static external variables that are declared but ...
>
Ahh, I see the problem. I was talking about "automatic variables" and
"static external variables" implicitly, both of which emit compiler
warnings if they are unreferenced as we saw in the arch/i386/pci/init.c
case and the example above, respectively.
The only advantage this gives us as opposed to just removing an
unreferenced variable completely is the case when its usage depends on
multiple preprocessor macros. Like in the arch/i386/pci/init.c example,
the use of 'type' depends on CONFIG_PCI_DIRECT or CONFIG_PCI_MMCONFIG. It
gets sloppy pretty quickly when we're adding
#if defined(CONFIG_PCI_DIRECT) || defined(CONFIG_PCI_MMCONFIG)
around the variable declarations. Unfortunately we still have the
drawback of perhaps leaving abandoned declarations in the code but now
there will be an easy way to detect it: defining __maybe_unused and __used
to be empty. Then we won't have hardcoded __attribute__ ((unused)) text
sprinkled around the tree that would require examination on an individual
basis.
David
On Tue, May 01, 2007 at 09:28:56PM -0700, David Rientjes wrote:
> There is no such thing as labeling a variable as __attribute__((used)).
> Since ts_shift is not referenced in inline assembly, we assume that we're
> simply suppressing a warning here if the variable is declared but
> unreferenced.
>
> Cc: Paul Mundt <[email protected]>
> Signed-off-by: David Rientjes <[email protected]>
Acked-by: Paul Mundt <[email protected]>