[announce] new tree: "fix all build warnings, on all configs" - Kernel

This is a discussion on [announce] new tree: "fix all build warnings, on all configs" - Kernel ; I'd like to announce a new upstream -git based tree i started working on some time ago, the "remove all compiler warnings on x86, on any .config" tree. This tree adds the CONFIG_ALLOW_WARNINGS config option - which, if disabled, adds ...

+ Reply to Thread
Page 1 of 2 1 2 LastLast
Results 1 to 20 of 23

Thread: [announce] new tree: "fix all build warnings, on all configs"

  1. [announce] new tree: "fix all build warnings, on all configs"


    I'd like to announce a new upstream -git based tree i started working on
    some time ago, the "remove all compiler warnings on x86, on any .config"
    tree.

    This tree adds the CONFIG_ALLOW_WARNINGS config option - which, if
    disabled, adds -Werror to the build flags and results in a failed build
    if any compiler warning is emitted by the kernel build. I started the
    tree based on David Howells's CONFIG_ENABLE_WERROR patchset.

    Right now the results are pretty good already: x86 builds with zero
    warnings both allyesconfig, allnoconfig and allmodconfig, 32-bit and
    64-bit as well - and most randconfig kernels will build fine as well. I
    continue fixing randconfig triggered warnings as i trigger them.

    The main purpose of this tree is to use it in the -tip auto-testing
    randconfig based kernel testing machinery, and to fix all new warnings
    that trigger via that patch influx vector - and to help other
    maintainers fix warnings that reach the upstream kernel.

    The tip/warnings tree consists of 5 topic branches comprising 83 commits
    at the moment:

    earth4:~/tip> tip-pending warnings/

    topic #commits
    -------------------------------------------
    warnings/bug : 19
    warnings/complex : 27
    warnings/infrastructure : 5
    warnings/simple : 23
    warnings/ugly : 9
    ---------------------
    total topics: 5
    pending topics: 5
    pending commits: 83

    tip/warnings/infrastructure:

    this topic contains the basic patches that enable this mode of
    building.

    tip/warnings/simple:

    this topic is for simple, obvious (looking) fixes.

    tip/warnings/complex:

    this topic is for fixes that i categorized as more complex -
    they need review and more testing.

    tip/warnings/ugly:

    these are workarounds for gcc bugs that i did not consider upstream
    worthy in any way. They also contain patches that fell through my
    review and need further improvements.

    tip/warnings/bug:

    this is a special topic for a certain type of warning: BUG() turning
    into a NOP on CONFIG_EMBEDDED + !CONFIG_BUG. In this case gcc is
    completely right in warning us in various places that we are
    triggering undefined behavior. This topic is fairly complete in that
    it fixes all these warnings i could trigger on
    allyesconfig+!CONFIG_BUG - but i'm on two minds what we should do. A
    simple solution would be to not allow the turning off of BUG() - but
    to turn it into something really simple, such as an infinite loop.
    Unfortunately that increases the size of the kernel by +0.2% so i did
    not want t do it without consideration. I kept these commits filtered
    out.

    Note that deprecated API and unused-return-value warnings
    (ENABLE_WARN_DEPRECATED and ENABLE_MUST_CHECK) are not included right
    now, there's way too many of them. If this effort works out fine the we
    could start covering those warnings too.

    It is also not clear to me how i should annotate variables for gcc false
    positives. The current upstream kernel method is:

    uninitialized_var(i);

    But Alan suggested that we should use __attribute__((used)) instead:

    #define __used __attribute__((used))

    and that he would not accept uninitialized_var() annotations. A central
    policy decision is needed on that. I wouldnt mind to convert all
    uninitialized_var() annotations over into __used annotations - but the
    question is: is this really an equivalent transformation? Can gcc be
    fooled into not initializing an incorrect __used annotation? With
    uninitialized_var() we at least have a specific initialization to zero.

    The integrated tree is available at tip/auto-warnings-next. Obviously
    the warnings/ugly and warnings/complex topics are still work in
    progress. The coordinates of the tip/warnings/simple topic can be found
    below.

    Comments, suggestions are welcome.

    Ingo

    -------------->
    the latest warnings/simple git tree is available at:

    git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git warnings/simple

    Ingo

    ------------------>
    Ingo Molnar (24):
    [vfs] fs.h - fix warning in drivers/infiniband/core/uverbs_main.c
    fix warning in fs/ext4/extents.c
    work around warning in fs/xfs/xfs_rtalloc.c
    fix warning in drivers/net/mlx4/mcg.c
    fix warning in drivers/net/mlx4/profile.c
    fix warning in arch/x86/kernel/io_apic_32.c
    fix warning in arch/x86/kernel/setup.c
    fix warning in sound/soc/codecs/tlv320aic23.c
    fix warning in drivers/ide/pci/hpt366.c
    fix warning in net/mac80211/rc80211_minstrel_debugfs.c
    fix warning in drivers/media/dvb/frontends/cx24116.c
    fix warning in drivers/video/cirrusfb.c
    fix warning in drivers/isdn/i4l/isdn_ppp.c
    fix warning in fs/afs/dir.c
    fix warning in net/netlabel/netlabel_addrlist.c
    fix warning in drivers/net/wireless/b43/phy_g.c
    fix warning in arch/x86/kernel/early-quirks.c
    include/linux/fs.h: fix warning in fs/gfs2/ops_file.c
    fix warning in drivers/base/platform.c
    fix warning in drivers/pci/pci-driver.c
    fix warning in drivers/acpi/sleep/main.c
    fix warning in net/ax25/sysctl_net_ax25.c
    fix warning in arch/x86/kernel/paravirt-spinlocks.c
    fix warnings in drivers/acpi/sbs.c


    arch/x86/kernel/early-quirks.c | 4 +++-
    arch/x86/kernel/io_apic_32.c | 4 ++--
    arch/x86/kernel/paravirt-spinlocks.c | 3 ++-
    arch/x86/kernel/setup.c | 2 ++
    drivers/acpi/sbs.c | 7 +++++++
    drivers/acpi/sleep/main.c | 10 +++++-----
    drivers/base/platform.c | 10 ++++++----
    drivers/ide/pci/hpt366.c | 1 -
    drivers/isdn/i4l/isdn_ppp.c | 2 ++
    drivers/media/dvb/frontends/cx24116.c | 3 ++-
    drivers/net/mlx4/mcg.c | 2 +-
    drivers/net/mlx4/profile.c | 2 ++
    drivers/net/wireless/b43/b43.h | 3 ++-
    drivers/pci/pci-driver.c | 9 +++++----
    drivers/video/cirrusfb.c | 3 +--
    fs/afs/dir.c | 2 +-
    fs/ext4/extents.c | 1 +
    fs/xfs/xfs_rtalloc.c | 2 +-
    include/linux/audit.h | 3 ++-
    include/linux/fs.h | 6 +++---
    net/ax25/sysctl_net_ax25.c | 2 ++
    net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
    sound/soc/codecs/tlv320aic23.c | 2 --
    23 files changed, 53 insertions(+), 32 deletions(-)

    diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
    index 733c4f8..e587947 100644
    --- a/arch/x86/kernel/early-quirks.c
    +++ b/arch/x86/kernel/early-quirks.c
    @@ -95,6 +95,7 @@ static void __init nvidia_bugs(int num, int slot, int func)

    }

    +#if defined(CONFIG_ACPI) && defined(CONFIG_X86_IO_APIC)
    static u32 ati_ixp4x0_rev(int num, int slot, int func)
    {
    u32 d;
    @@ -112,10 +113,11 @@ static u32 ati_ixp4x0_rev(int num, int slot, int func)
    d &= 0xff;
    return d;
    }
    +#endif

    static void __init ati_bugs(int num, int slot, int func)
    {
    -#if defined(CONFIG_ACPI) && defined (CONFIG_X86_IO_APIC)
    +#if defined(CONFIG_ACPI) && defined(CONFIG_X86_IO_APIC)
    u32 d;
    u8 b;

    diff --git a/arch/x86/kernel/io_apic_32.c b/arch/x86/kernel/io_apic_32.c
    index e710289..64f0953 100644
    --- a/arch/x86/kernel/io_apic_32.c
    +++ b/arch/x86/kernel/io_apic_32.c
    @@ -1536,8 +1536,8 @@ __apicdebuginit(void) print_local_APIC(void *dummy)
    }

    icr = apic_icr_read();
    - printk(KERN_DEBUG "... APIC ICR: %08x\n", icr);
    - printk(KERN_DEBUG "... APIC ICR2: %08x\n", icr >> 32);
    + printk(KERN_DEBUG "... APIC ICR: %08x\n", (u32)icr);
    + printk(KERN_DEBUG "... APIC ICR2: %08x\n", (u32)(icr >> 32));

    v = apic_read(APIC_LVTT);
    printk(KERN_DEBUG "... APIC LVTT: %08x\n", v);
    diff --git a/arch/x86/kernel/paravirt-spinlocks.c b/arch/x86/kernel/paravirt-spinlocks.c
    index 0e9f198..95777b0 100644
    --- a/arch/x86/kernel/paravirt-spinlocks.c
    +++ b/arch/x86/kernel/paravirt-spinlocks.c
    @@ -7,7 +7,8 @@

    #include

    -static void default_spin_lock_flags(struct raw_spinlock *lock, unsigned long flags)
    +static inline void
    +default_spin_lock_flags(raw_spinlock_t *lock, unsigned long flags)
    {
    __raw_spin_lock(lock);
    }
    diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
    index 2255782..44c2315 100644
    --- a/arch/x86/kernel/setup.c
    +++ b/arch/x86/kernel/setup.c
    @@ -732,6 +732,7 @@ void start_periodic_check_for_corruption(void)
    }
    #endif

    +#ifdef CONFIG_X86_RESERVE_LOW_64K
    static int __init dmi_low_memory_corruption(const struct dmi_system_id *d)
    {
    printk(KERN_NOTICE
    @@ -743,6 +744,7 @@ static int __init dmi_low_memory_corruption(const struct dmi_system_id *d)

    return 0;
    }
    +#endif

    /* List of systems that have known low memory corruption BIOS problems */
    static struct dmi_system_id __initdata bad_bios_dmi_table[] = {
    diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
    index 10a3651..2657182 100644
    --- a/drivers/acpi/sbs.c
    +++ b/drivers/acpi/sbs.c
    @@ -389,6 +389,8 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
    return result;
    }

    +#if defined(CONFIG_ACPI_SYSFS_POWER) || defined(CONFIG_ACPI_PROCFS_POWER)
    +
    static int acpi_battery_get_alarm(struct acpi_battery *battery)
    {
    return acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD,
    @@ -425,6 +427,8 @@ static int acpi_battery_set_alarm(struct acpi_battery *battery)
    return ret;
    }

    +#endif
    +
    static int acpi_ac_get_present(struct acpi_sbs *sbs)
    {
    int result;
    @@ -816,7 +820,10 @@ static int acpi_battery_add(struct acpi_sbs *sbs, int id)

    static void acpi_battery_remove(struct acpi_sbs *sbs, int id)
    {
    +#if defined(CONFIG_ACPI_SYSFS_POWER) || defined(CONFIG_ACPI_PROCFS_POWER)
    struct acpi_battery *battery = &sbs->battery[id];
    +#endif
    +
    #ifdef CONFIG_ACPI_SYSFS_POWER
    if (battery->bat.dev) {
    if (battery->have_sysfs_alarm)
    diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
    index d13194a..2276d75 100644
    --- a/drivers/acpi/sleep/main.c
    +++ b/drivers/acpi/sleep/main.c
    @@ -63,7 +63,7 @@ void __init acpi_old_suspend_ordering(void)
    /**
    * acpi_pm_disable_gpes - Disable the GPEs.
    */
    -static int acpi_pm_disable_gpes(void)
    +static inline int acpi_pm_disable_gpes(void)
    {
    acpi_hw_disable_all_gpes();
    return 0;
    @@ -75,7 +75,7 @@ static int acpi_pm_disable_gpes(void)
    * If necessary, set the firmware waking vector and do arch-specific
    * nastiness to get the wakeup code to the waking vector.
    */
    -static int __acpi_pm_prepare(void)
    +static inline int __acpi_pm_prepare(void)
    {
    int error = acpi_sleep_prepare(acpi_target_sleep_state);

    @@ -88,7 +88,7 @@ static int __acpi_pm_prepare(void)
    * acpi_pm_prepare - Prepare the platform to enter the target sleep
    * state and disable the GPEs.
    */
    -static int acpi_pm_prepare(void)
    +static inline int acpi_pm_prepare(void)
    {
    int error = __acpi_pm_prepare();

    @@ -103,7 +103,7 @@ static int acpi_pm_prepare(void)
    * This is called after we wake back up (or if entering the sleep state
    * failed).
    */
    -static void acpi_pm_finish(void)
    +static inline void acpi_pm_finish(void)
    {
    u32 acpi_state = acpi_target_sleep_state;

    @@ -124,7 +124,7 @@ static void acpi_pm_finish(void)
    /**
    * acpi_pm_end - Finish up suspend sequence.
    */
    -static void acpi_pm_end(void)
    +static inline void acpi_pm_end(void)
    {
    /*
    * This is necessary in case acpi_pm_finish() is not called during a
    diff --git a/drivers/base/platform.c b/drivers/base/platform.c
    index dfcbfe5..e0bcd6b 100644
    --- a/drivers/base/platform.c
    +++ b/drivers/base/platform.c
    @@ -614,7 +614,8 @@ static int platform_match(struct device *dev, struct device_driver *drv)

    #ifdef CONFIG_PM_SLEEP

    -static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
    +static inline int
    +platform_legacy_suspend(struct device *dev, pm_message_t mesg)
    {
    int ret = 0;

    @@ -624,7 +625,8 @@ static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
    return ret;
    }

    -static int platform_legacy_suspend_late(struct device *dev, pm_message_t mesg)
    +static inline int
    +platform_legacy_suspend_late(struct device *dev, pm_message_t mesg)
    {
    struct platform_driver *drv = to_platform_driver(dev->driver);
    struct platform_device *pdev;
    @@ -637,7 +639,7 @@ static int platform_legacy_suspend_late(struct device *dev, pm_message_t mesg)
    return ret;
    }

    -static int platform_legacy_resume_early(struct device *dev)
    +static inline int platform_legacy_resume_early(struct device *dev)
    {
    struct platform_driver *drv = to_platform_driver(dev->driver);
    struct platform_device *pdev;
    @@ -650,7 +652,7 @@ static int platform_legacy_resume_early(struct device *dev)
    return ret;
    }

    -static int platform_legacy_resume(struct device *dev)
    +static inline int platform_legacy_resume(struct device *dev)
    {
    int ret = 0;

    diff --git a/drivers/ide/pci/hpt366.c b/drivers/ide/pci/hpt366.c
    index 9cf171c..ca0acb5 100644
    --- a/drivers/ide/pci/hpt366.c
    +++ b/drivers/ide/pci/hpt366.c
    @@ -1289,7 +1289,6 @@ static u8 hpt3xx_cable_detect(ide_hwif_t *hwif)

    static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
    {
    - struct pci_dev *dev = to_pci_dev(hwif->dev);
    struct hpt_info *info = hpt3xx_get_info(hwif->dev);
    int serialize = HPT_SERIALIZE_IO;
    u8 chip_type = info->chip_type;
    diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
    index 77c280e..c179360 100644
    --- a/drivers/isdn/i4l/isdn_ppp.c
    +++ b/drivers/isdn/i4l/isdn_ppp.c
    @@ -431,6 +431,7 @@ set_arg(void __user *b, void *val,int len)
    return 0;
    }

    +#ifdef CONFIG_IPPP_FILTER
    static int get_filter(void __user *arg, struct sock_filter **p)
    {
    struct sock_fprog uprog;
    @@ -465,6 +466,7 @@ static int get_filter(void __user *arg, struct sock_filter **p)
    *p = code;
    return uprog.len;
    }
    +#endif

    /*
    * ippp device ioctl
    diff --git a/drivers/media/dvb/frontends/cx24116.c b/drivers/media/dvb/frontends/cx24116.c
    index deb36f4..eb5febc 100644
    --- a/drivers/media/dvb/frontends/cx24116.c
    +++ b/drivers/media/dvb/frontends/cx24116.c
    @@ -200,7 +200,8 @@ static int cx24116_writereg(struct cx24116_state* state, int reg, int data)
    }

    /* Bulk byte writes to a single I2C address, for 32k firmware load */
    -static int cx24116_writeregN(struct cx24116_state* state, int reg, u8 *data, u16 len)
    +static int
    +cx24116_writeregN(struct cx24116_state* state, int reg, const u8 *data, u16 len)
    {
    int ret = -EREMOTEIO;
    struct i2c_msg msg;
    diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
    index c83f88c..d1230a8 100644
    --- a/drivers/net/mlx4/mcg.c
    +++ b/drivers/net/mlx4/mcg.c
    @@ -215,7 +215,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],

    if (block_mcast_loopback)
    mgm->qp[members_count++] = cpu_to_be32((qp->qpn & MGM_QPN_MASK) |
    - (1 << MGM_BLCK_LB_BIT));
    + (1U << MGM_BLCK_LB_BIT));
    else
    mgm->qp[members_count++] = cpu_to_be32(qp->qpn & MGM_QPN_MASK);

    diff --git a/drivers/net/mlx4/profile.c b/drivers/net/mlx4/profile.c
    index 9ca42b2..ec9383d 100644
    --- a/drivers/net/mlx4/profile.c
    +++ b/drivers/net/mlx4/profile.c
    @@ -52,6 +52,7 @@ enum {
    MLX4_RES_NUM
    };

    +#ifdef CONFIG_MLX4_DEBUG
    static const char *res_name[] = {
    [MLX4_RES_QP] = "QP",
    [MLX4_RES_RDMARC] = "RDMARC",
    @@ -65,6 +66,7 @@ static const char *res_name[] = {
    [MLX4_RES_MTT] = "MTT",
    [MLX4_RES_MCG] = "MCG",
    };
    +#endif

    u64 mlx4_make_profile(struct mlx4_dev *dev,
    struct mlx4_profile *request,
    diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
    index 427b820..ac55b62 100644
    --- a/drivers/net/wireless/b43/b43.h
    +++ b/drivers/net/wireless/b43/b43.h
    @@ -853,7 +853,8 @@ void b43warn(struct b43_wl *wl, const char *fmt, ...)
    void b43dbg(struct b43_wl *wl, const char *fmt, ...)
    __attribute__ ((format(printf, 2, 3)));
    #else /* DEBUG */
    -# define b43dbg(wl, fmt...) do { /* nothing */ } while (0)
    +static inline void __attribute__ ((format(printf, 2, 3)))
    +b43dbg(struct b43_wl *wl, const char *fmt, ...) { }
    #endif /* DEBUG */

    /* A WARN_ON variant that vanishes when b43 debugging is disabled.
    diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
    index a13f534..b062868 100644
    --- a/drivers/pci/pci-driver.c
    +++ b/drivers/pci/pci-driver.c
    @@ -324,7 +324,7 @@ static int pci_default_pm_resume(struct pci_dev *pci_dev)
    return retval;
    }

    -static int pci_legacy_suspend(struct device *dev, pm_message_t state)
    +static inline int pci_legacy_suspend(struct device *dev, pm_message_t state)
    {
    struct pci_dev * pci_dev = to_pci_dev(dev);
    struct pci_driver * drv = pci_dev->driver;
    @@ -339,7 +339,8 @@ static int pci_legacy_suspend(struct device *dev, pm_message_t state)
    return i;
    }

    -static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
    +static inline int
    +pci_legacy_suspend_late(struct device *dev, pm_message_t state)
    {
    struct pci_dev * pci_dev = to_pci_dev(dev);
    struct pci_driver * drv = pci_dev->driver;
    @@ -352,7 +353,7 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
    return i;
    }

    -static int pci_legacy_resume(struct device *dev)
    +static inline int pci_legacy_resume(struct device *dev)
    {
    int error;
    struct pci_dev * pci_dev = to_pci_dev(dev);
    @@ -365,7 +366,7 @@ static int pci_legacy_resume(struct device *dev)
    return error;
    }

    -static int pci_legacy_resume_early(struct device *dev)
    +static inline int pci_legacy_resume_early(struct device *dev)
    {
    int error = 0;
    struct pci_dev * pci_dev = to_pci_dev(dev);
    diff --git a/drivers/video/cirrusfb.c b/drivers/video/cirrusfb.c
    index 048b139..ace4001 100644
    --- a/drivers/video/cirrusfb.c
    +++ b/drivers/video/cirrusfb.c
    @@ -2462,8 +2462,7 @@ static int __init cirrusfb_init(void)

    #ifndef MODULE
    static int __init cirrusfb_setup(char *options) {
    - char *this_opt, s[32];
    - int i;
    + char *this_opt;

    DPRINTK("ENTER\n");

    diff --git a/fs/afs/dir.c b/fs/afs/dir.c
    index dfda03d..254c567 100644
    --- a/fs/afs/dir.c
    +++ b/fs/afs/dir.c
    @@ -563,7 +563,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
    static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
    {
    struct afs_vnode *vnode, *dir;
    - struct afs_fid fid;
    + struct afs_fid fid = { 0, };
    struct dentry *parent;
    struct key *key;
    void *dir_version;
    diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
    index ea2ce3c..9a34682 100644
    --- a/fs/ext4/extents.c
    +++ b/fs/ext4/extents.c
    @@ -1156,6 +1156,7 @@ ext4_ext_search_right(struct inode *inode, struct ext4_ext_path *path,
    return 0;
    }

    + ix = NULL; /* avoid gcc false positive warning */
    /* go up and search for index to the right */
    while (--depth >= 0) {
    ix = path[depth].p_idx;
    diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
    index e2f68de..fe5de08 100644
    --- a/fs/xfs/xfs_rtalloc.c
    +++ b/fs/xfs/xfs_rtalloc.c
    @@ -1872,7 +1872,7 @@ xfs_growfs_rt(
    xfs_extlen_t rsumblocks; /* current number of rt summary blks */
    xfs_sb_t *sbp; /* old superblock */
    xfs_fsblock_t sumbno; /* summary block number */
    - xfs_trans_t *tp; /* transaction pointer */
    + xfs_trans_t *uninitialized_var(tp); /* transaction pointer */

    sbp = &mp->m_sb;
    cancelflags = 0;
    diff --git a/include/linux/audit.h b/include/linux/audit.h
    index 6272a39..a3f78d0 100644
    --- a/include/linux/audit.h
    +++ b/include/linux/audit.h
    @@ -580,7 +580,8 @@ extern int audit_enabled;
    #define audit_log(c,g,t,f,...) do { ; } while (0)
    #define audit_log_start(c,g,t) ({ NULL; })
    #define audit_log_vformat(b,f,a) do { ; } while (0)
    -#define audit_log_format(b,f,...) do { ; } while (0)
    +static inline void __attribute__ ((format(printf, 2, 3)))
    +audit_log_format(struct audit_buffer *ab, const char *fmt, ...) { }
    #define audit_log_end(b) do { ; } while (0)
    #define audit_log_n_hex(a,b,l) do { ; } while (0)
    #define audit_log_n_string(a,c,l) do { ; } while (0)
    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index a6a625b..114f469 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -1597,9 +1597,9 @@ void unnamed_dev_init(void);

    /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
    #define fops_get(fops) \
    - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
    + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL))
    #define fops_put(fops) \
    - do { if (fops) module_put((fops)->owner); } while(0)
    + do { if (fops != NULL) module_put((fops)->owner); } while(0)

    extern int register_filesystem(struct file_system_type *);
    extern int unregister_filesystem(struct file_system_type *);
    @@ -1675,7 +1675,7 @@ static inline int break_lease(struct inode *inode, unsigned int mode)
    #else /* !CONFIG_FILE_LOCKING */
    #define locks_mandatory_locked(a) ({ 0; })
    #define locks_mandatory_area(a, b, c, d, e) ({ 0; })
    -#define __mandatory_lock(a) ({ 0; })
    +static inline int __mandatory_lock(struct inode *ino) { return 0; }
    #define mandatory_lock(a) ({ 0; })
    #define locks_verify_locked(a) ({ 0; })
    #define locks_verify_truncate(a, b, c) ({ 0; })
    diff --git a/net/ax25/sysctl_net_ax25.c b/net/ax25/sysctl_net_ax25.c
    index f288fc4..735ceef 100644
    --- a/net/ax25/sysctl_net_ax25.c
    +++ b/net/ax25/sysctl_net_ax25.c
    @@ -24,7 +24,9 @@ static int min_idle[1], max_idle[] = {65535000};
    static int min_n2[] = {1}, max_n2[] = {31};
    static int min_paclen[] = {1}, max_paclen[] = {512};
    static int min_proto[1], max_proto[] = { AX25_PROTO_MAX };
    +#ifdef CONFIG_AX25_DAMA_SLAVE
    static int min_ds_timeout[1], max_ds_timeout[] = {65535000};
    +#endif

    static struct ctl_table_header *ax25_table_header;

    diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
    index 0b024cd..26f8ffc 100644
    --- a/net/mac80211/rc80211_minstrel_debugfs.c
    +++ b/net/mac80211/rc80211_minstrel_debugfs.c
    @@ -106,7 +106,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
    return 0;
    }

    -static int
    +static ssize_t
    minstrel_stats_read(struct file *file, char __user *buf, size_t len, loff_t *o)
    {
    struct minstrel_stats_info *ms;
    diff --git a/sound/soc/codecs/tlv320aic23.c b/sound/soc/codecs/tlv320aic23.c
    index 44308da..45395d0 100644
    --- a/sound/soc/codecs/tlv320aic23.c
    +++ b/sound/soc/codecs/tlv320aic23.c
    @@ -421,8 +421,6 @@ static int tlv320aic23_set_dai_fmt(struct snd_soc_dai *codec_dai,
    static int tlv320aic23_set_dai_sysclk(struct snd_soc_dai *codec_dai,
    int clk_id, unsigned int freq, int dir)
    {
    - struct snd_soc_codec *codec = codec_dai->codec;
    -
    switch (freq) {
    case 12000000:
    return 0;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Roland Dreier wrote:

    > > diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
    > > index c83f88c..d1230a8 100644
    > > --- a/drivers/net/mlx4/mcg.c
    > > +++ b/drivers/net/mlx4/mcg.c
    > > @@ -215,7 +215,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],
    > >
    > > if (block_mcast_loopback)
    > > mgm->qp[members_count++] = cpu_to_be32((qp->qpn & MGM_QPN_MASK) |
    > > - (1 << MGM_BLCK_LB_BIT));
    > > + (1U << MGM_BLCK_LB_BIT));

    >
    > This is fixing a warning caused by a gcc bug
    > (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37261) which is fixed
    > upstream. The change is rather inoffensive but on the other hand I'm
    > not sure what our policy for dealing with version-specific warning
    > bugs in gcc should be.


    the full commit is below - i researched it when i created it and indeed
    it seemed like an incorrect warning.

    OTOH, there should be a well-defined work flow to keep this all
    manageable: once we know why a warning triggers and it has been
    categorized by a human, we should get rid of the warning in some way.

    Applying this patch as-is would be one option. Annotating it with a
    specific gcc version would be overkill i think. Ignoring it would be
    bad, because there's real value in standardizing on a "no warnings"
    build output. Many new warnings get introduced because people do not
    notice new warnings amongst the very high baseline noise of the kernel
    build.

    What do you think?

    Ingo

    --------->
    From 367bb845bef83d64cfee452a18a84fd65f21d401 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Mon, 18 Aug 2008 16:18:34 +0200
    Subject: [PATCH] fix warning in drivers/net/mlx4/mcg.c
    MIME-Version: 1.0
    Content-Type: text/plain; charset=utf-8
    Content-Transfer-Encoding: 8bit

    fix warning:

    drivers/net/mlx4/mcg.c: In function ‘mlx4_multicast_attach’:
    drivers/net/mlx4/mcg.c:217: warning: integer overflow in expression

    there was no real danger of overflow here though.

    md5:
    db8eb55620f886c03854a2abb2ce6c3f mcg.o.before.asm
    db8eb55620f886c03854a2abb2ce6c3f mcg.o.after.asm

    Signed-off-by: Ingo Molnar
    ---
    drivers/net/mlx4/mcg.c | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
    index c83f88c..d1230a8 100644
    --- a/drivers/net/mlx4/mcg.c
    +++ b/drivers/net/mlx4/mcg.c
    @@ -215,7 +215,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],

    if (block_mcast_loopback)
    mgm->qp[members_count++] = cpu_to_be32((qp->qpn & MGM_QPN_MASK) |
    - (1 << MGM_BLCK_LB_BIT));
    + (1U << MGM_BLCK_LB_BIT));
    else
    mgm->qp[members_count++] = cpu_to_be32(qp->qpn & MGM_QPN_MASK);

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [announce] new tree: "fix all build warnings, on all configs"

    > diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
    > index c83f88c..d1230a8 100644
    > --- a/drivers/net/mlx4/mcg.c
    > +++ b/drivers/net/mlx4/mcg.c
    > @@ -215,7 +215,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],
    >
    > if (block_mcast_loopback)
    > mgm->qp[members_count++] = cpu_to_be32((qp->qpn & MGM_QPN_MASK) |
    > - (1 << MGM_BLCK_LB_BIT));
    > + (1U << MGM_BLCK_LB_BIT));


    This is fixing a warning caused by a gcc bug
    (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37261) which is fixed
    upstream. The change is rather inoffensive but on the other hand I'm
    not sure what our policy for dealing with version-specific warning bugs
    in gcc should be.

    - R.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [announce] new tree: "fix all build warnings, on all configs"

    > OTOH, there should be a well-defined work flow to keep this all
    > manageable: once we know why a warning triggers and it has been
    > categorized by a human, we should get rid of the warning in some way.
    >
    > Applying this patch as-is would be one option. Annotating it with a
    > specific gcc version would be overkill i think. Ignoring it would be
    > bad, because there's real value in standardizing on a "no warnings"
    > build output. Many new warnings get introduced because people do not
    > notice new warnings amongst the very high baseline noise of the kernel
    > build.


    The specific change I noticed:

    > - (1 << MGM_BLCK_LB_BIT));
    > + (1U << MGM_BLCK_LB_BIT));


    is not a problem to me -- the code is fine either way, and if we're
    making an effort to kill all warnings, then I'm OK with merging it.
    It's a little unfortunate to add churn due to a gcc bug that is only in
    certain 4.3 releases, but this particular case doesn't seem to trigger
    in many places, so the cost is low.

    However I worry about warnings produced by gcc bugs forcing us to tinker
    with correct code. Maybe it just makes sense to wait and see if we ever
    hit a case where a gcc bug forces us to make too many stupid changes,
    and figure out what to do if and when that happens.

    - R.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Roland Dreier wrote:

    > However I worry about warnings produced by gcc bugs forcing us to
    > tinker with correct code. Maybe it just makes sense to wait and see
    > if we ever hit a case where a gcc bug forces us to make too many
    > stupid changes, and figure out what to do if and when that happens.


    i certainly have a found a couple of such cases, see tip/warnings/ugly -
    for example see the one below where gcc is not able to see through type
    width.

    the threshold i'm using is: "does the code get worse". Another question
    is the rate of really ugly patches. Had i ended up with a lot of them
    i'd be worried - but right now they are in the low single digits, for a
    full 9 MLOC kernel - which seems manageable.

    the drivers/net/mlx4/mcg.c commit you pointed out is one of the very few
    borderline cases: the code gets neither better, nor worse. If you look
    at the totality of fixes they are not common at all. (and almost by
    definition the 100-200 unfixed warnings that we have piled up in -git
    are the _problematic_ cases - clear-cut cases tend to be fixed.)

    Ingo

    ------------->
    From fbf03326a16b29f8d34a5a3883a267bac4d38fc2 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Wed, 27 Aug 2008 12:44:00 +0200
    Subject: [PATCH] hack, workaround for warning drivers/acpi/tables/tbfadt.c
    MIME-Version: 1.0
    Content-Type: text/plain; charset=utf-8
    Content-Transfer-Encoding: 8bit

    ugly workaround for this warning:

    drivers/acpi/tables/tbfadt.c: In function ‘acpi_tb_create_local_fadt’:
    include/asm/string_32.h:75: warning: array subscript is above array bounds

    gcc 4.3.1 20080801 (Red Hat 4.3.1-6)

    its array checks are borked. Switch the string_32.h code to short instead.

    NOT-Signed-off-by: Ingo Molnar
    ---
    include/asm-x86/string_32.h | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/include/asm-x86/string_32.h b/include/asm-x86/string_32.h
    index 487843e..419ab10 100644
    --- a/include/asm-x86/string_32.h
    +++ b/include/asm-x86/string_32.h
    @@ -72,7 +72,7 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
    return to;
    case 5:
    *(int *)to = *(int *)from;
    - *((char *)to + 4) = *((char *)from + 4);
    + *((short *)to + 3) = *((short *)from + 3);
    return to;
    case 6:
    *(int *)to = *(int *)from;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [announce] new tree: "fix all build warnings, on all configs"

    > the drivers/net/mlx4/mcg.c commit you pointed out is one of the very few
    > borderline cases: the code gets neither better, nor worse.


    Yes, I agree exactly. As long as there are not too many such cases
    (since every commit has some cost just from causing churn) then we are
    OK, I think.

    > If you look at the totality of fixes they are not common at all. (and
    > almost by definition the 100-200 unfixed warnings that we have piled
    > up in -git are the _problematic_ cases - clear-cut cases tend to be
    > fixed.)


    Yes, and I think that merging such changes makes the most sense as part
    of a project such as yours that wants to kill all warnings. I looked at
    the mcg.c warning and found the same workaround, but in the context of
    my maintenance work, I just reported the gcc bug and lived with the
    warning when using gcc 4.3.

    By the way, just out of curiousity, how are you dealing with warnings
    about "format not a string literal and no format arguments" caused by
    code like arch/x86/kernel/dumpstack_64.c:

    static void print_trace_address(void *data, unsigned long addr, int reliable)
    {
    touch_nmi_watchdog();
    printk(data);
    printk_address(addr, reliable);
    }

    and also cases like:

    char *name;

    //...

    kobject_set_name(obj, name);

    (I get these with gcc "(Ubuntu 4.3.2-1ubuntu10) 4.3.2")

    > i certainly have a found a couple of such cases, see tip/warnings/ugly -
    > for example see the one below where gcc is not able to see through type
    > width.


    Yes, the uninitialized variable warnings are obnoxious too. By the way,
    I think this:

    @@ -72,7 +72,7 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
    return to;
    case 5:
    *(int *)to = *(int *)from;
    - *((char *)to + 4) = *((char *)from + 4);
    + *((short *)to + 3) = *((short *)from + 3);
    return to;
    case 6:
    *(int *)to = *(int *)from;

    is actually *wrong*, because the cast operator binds tighter than
    addition -- so

    + *((short *)to + 3) = *((short *)from + 3);

    actually copies bytes at offset 6 and 7; I think what you intended was:

    + *((short *)(to + 3)) = *((short *)(from + 3));

    which illustrates the risks in fixing warnings.

    - R.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [announce] new tree: "fix all build warnings, on all configs"

    Ingo Molnar writes:
    > if (battery->have_sysfs_alarm)
    > diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
    > index d13194a..2276d75 100644
    > --- a/drivers/acpi/sleep/main.c
    > +++ b/drivers/acpi/sleep/main.c
    > @@ -63,7 +63,7 @@ void __init acpi_old_suspend_ordering(void)
    > /**
    > * acpi_pm_disable_gpes - Disable the GPEs.
    > */
    > -static int acpi_pm_disable_gpes(void)
    > +static inline int acpi_pm_disable_gpes(void)


    Just to satisfy my curiosity, what compiler warning does marking functions inline
    fix?

    Thanks.

    -Andi

    --
    ak@linux.intel.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Roland Dreier wrote:

    > > i certainly have a found a couple of such cases, see tip/warnings/ugly -
    > > for example see the one below where gcc is not able to see through type
    > > width.

    >
    > Yes, the uninitialized variable warnings are obnoxious too. By the way,
    > I think this:
    >
    > @@ -72,7 +72,7 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
    > return to;
    > case 5:
    > *(int *)to = *(int *)from;
    > - *((char *)to + 4) = *((char *)from + 4);
    > + *((short *)to + 3) = *((short *)from + 3);
    > return to;
    > case 6:
    > *(int *)to = *(int *)from;
    >
    > is actually *wrong*, because the cast operator binds tighter than
    > addition -- so
    >
    > + *((short *)to + 3) = *((short *)from + 3);
    >
    > actually copies bytes at offset 6 and 7; I think what you intended was:
    >
    > + *((short *)(to + 3)) = *((short *)(from + 3));


    thx, you are right - fixed it via the patch below.

    > which illustrates the risks in fixing warnings.


    yeah. Note that this was not a routine case at all, i did the commit in
    the early stages when i didnt even know how much effort it all would be
    to keep the whole kernel warning-free, in all configs. It looked odd and
    ugly and was in tip/warnings/ugly rightfully.

    It would be nice if you could find an outright incorrect change in
    tip/warnings/simple. The ones flagged 'simple' are the ones that have
    the highest risk of not being reviewed much beyond their initial
    addition.

    Ingo

    --------------->
    From 9d8f9578ca252bf26474ed77fde7ea30e9dee595 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Sat, 18 Oct 2008 10:17:36 +0200
    Subject: [PATCH] hack, workaround for warning drivers/acpi/tables/tbfadt.c, fix

    Fix commit fbf03326a16b29f8d34a5a3883a267bac4d38fc2, pointed
    out by Roland Dreier.

    Signed-off-by: Ingo Molnar
    ---
    include/asm-x86/string_32.h | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/include/asm-x86/string_32.h b/include/asm-x86/string_32.h
    index 419ab10..be82619 100644
    --- a/include/asm-x86/string_32.h
    +++ b/include/asm-x86/string_32.h
    @@ -72,7 +72,7 @@ static __always_inline void *__constant_memcpy(void *to, const void *from,
    return to;
    case 5:
    *(int *)to = *(int *)from;
    - *((short *)to + 3) = *((short *)from + 3);
    + *((char *)(to + 3)) = *((char *)(from + 3));
    return to;
    case 6:
    *(int *)to = *(int *)from;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [announce] new tree: "fix all build warnings, on all configs"



    On Sat, 18 Oct 2008, Ingo Molnar wrote:
    >
    > thx, you are right - fixed it via the patch below.


    Hell no.

    The old code was correct. Your code is ****. And you didn't fix
    _anything_.

    > case 5:
    > *(int *)to = *(int *)from;
    > - *((short *)to + 3) = *((short *)from + 3);
    > + *((char *)(to + 3)) = *((char *)(from + 3));
    > return to;


    Are you just making changes by randomly inserting and deleting characters
    until you don't see warnings? Or what?

    That thing is supposed to be a 5-byte memcpy. Not a "take a random byte
    from a random location and move it to another random location". That would
    be "randcpy()", not "memcpy()".

    I don't want to see obvious and ****ty crap like this. I don't want to
    pull from people who write code with some "random walk" algorithm.

    F*ck me, what's wrong with you people? THAT CODE WAS NOT BUGGY. If it
    causes a warning, it is because SOME CALLER used a 5-byte memcpy() on
    something that gcc thought was just four bytes in size.

    Ingo, I'm not going to pull _anything_ from you.

    Linus
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [announce] new tree: "fix all build warnings, on all configs"

    Linus Torvalds wrote:
    >
    > The old code was correct. Your code is ****. And you didn't fix
    > _anything_.
    >
    >> case 5:
    >> *(int *)to = *(int *)from;
    >> - *((short *)to + 3) = *((short *)from + 3);
    >> + *((char *)(to + 3)) = *((char *)(from + 3));
    >> return to;

    >
    > Are you just making changes by randomly inserting and deleting characters
    > until you don't see warnings? Or what?
    >
    > That thing is supposed to be a 5-byte memcpy. Not a "take a random byte
    > from a random location and move it to another random location". That would
    > be "randcpy()", not "memcpy()".
    >


    That is not a 5-byte memcopy. In *either* version!

    In the "before" case, it copies bytes 0, 1, 2, 3, 6 and 7.
    In the "after" case, it copies bytes 0, 1 and 2.

    Presumably it *should* be:

    *((char *)to + 4) = *((char *)from + 4);

    -hpa
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [announce] new tree: "fix all build warnings, on all configs"



    On Mon, 20 Oct 2008, H. Peter Anvin wrote:
    >
    > Presumably it *should* be:
    >
    > *((char *)to + 4) = *((char *)from + 4);


    That's what it is in the kenrel. The "before and after" versions were both
    from broken Ingo code.

    Linus
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Linus Torvalds wrote:

    > That thing is supposed to be a 5-byte memcpy. Not a "take a random
    > byte from a random location and move it to another random location".
    > That would be "randcpy()", not "memcpy()".
    >
    > I don't want to see obvious and ****ty crap like this. I don't want to
    > pull from people who write code with some "random walk" algorithm.


    yes, the patch is worse than crap, so i:

    1) signalled that it's crap in its branch name (warnings/ugly)
    2) named it a hack in the subject line: "hack, workaround for warning"
    3) explained it in the commit log that GCC is crap
    4) added a NOT-Signed-off

    but i guess i should not even have created it, because it shows a broken
    "symptom driven" thought process when it was created.

    So i zapped the 'ugly' branch completely and removed all those commits.
    Will filter out bogus warnings via different technical means. Have no
    ideas at the moment of how to do it technically though - filtering the
    compiler output does not work reliably.

    Below are the kind of commits i want to end up with eventually and
    reliably.

    Ingo

    ------------------>

    git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git warnings/simple

    Ingo Molnar (10):
    x86: fix default_spin_lock_flags() prototype
    drivers/ide/pci/hpt366.c: remove unused variable
    drivers/net/wireless/b43/phy_g.c: type check debug printouts
    drivers/video/cirrusfb.c: remove unused variables
    fs/afs/dir.c: fix uninitialized variable use
    net/netlabel/netlabel_addrlist.c: add type checking to audit_log_format()
    include/linux/fs.h: improve type checking of __mandatory_lock()
    [vfs] fs.h: fops_get()/fops_put(): use pointer comparison
    net/mac80211/rc80211_minstrel_debugfs.c: fix return type
    sound/soc/codecs/tlv320aic23.c: remove unused variable


    arch/x86/kernel/paravirt-spinlocks.c | 3 ++-
    drivers/ide/pci/hpt366.c | 1 -
    drivers/net/wireless/b43/b43.h | 3 ++-
    drivers/video/cirrusfb.c | 3 +--
    fs/afs/dir.c | 2 +-
    include/linux/audit.h | 3 ++-
    include/linux/fs.h | 6 +++---
    net/mac80211/rc80211_minstrel_debugfs.c | 2 +-
    sound/soc/codecs/tlv320aic23.c | 2 --
    9 files changed, 12 insertions(+), 13 deletions(-)

    commit 54b1d646d442289d8d49e04bc2f10ba122ff6aa4
    Author: Ingo Molnar
    Date: Fri Oct 17 12:07:47 2008 +0200

    sound/soc/codecs/tlv320aic23.c: remove unused variable

    this warning:

    sound/soc/codecs/tlv320aic23.c: In function ‘tlv320aic23_set_dai_sysclk’:
    sound/soc/codecs/tlv320aic23.c:424: warning: unused variable ‘codec’

    triggers because this variable is not used in any form.

    Remove it.

    Signed-off-by: Ingo Molnar

    diff --git a/sound/soc/codecs/tlv320aic23.c b/sound/soc/codecs/tlv320aic23.c
    index 44308da..45395d0 100644
    --- a/sound/soc/codecs/tlv320aic23.c
    +++ b/sound/soc/codecs/tlv320aic23.c
    @@ -421,8 +421,6 @@ static int tlv320aic23_set_dai_fmt(struct snd_soc_dai *codec_dai,
    static int tlv320aic23_set_dai_sysclk(struct snd_soc_dai *codec_dai,
    int clk_id, unsigned int freq, int dir)
    {
    - struct snd_soc_codec *codec = codec_dai->codec;
    -
    switch (freq) {
    case 12000000:
    return 0;

    commit 0be735b3ff71e13e24b82420f23a1b3b0a207ffb
    Author: Ingo Molnar
    Date: Fri Oct 17 12:56:23 2008 +0200

    net/mac80211/rc80211_minstrel_debugfs.c: fix return type

    this warning:

    net/mac80211/rc80211_minstrel_debugfs.c:145: warning: initialization from incompatible pointer type

    triggers because the proper return type for file_operations::read
    is ssize_t, not int.

    Signed-off-by: Ingo Molnar

    diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
    index 0b024cd..26f8ffc 100644
    --- a/net/mac80211/rc80211_minstrel_debugfs.c
    +++ b/net/mac80211/rc80211_minstrel_debugfs.c
    @@ -106,7 +106,7 @@ minstrel_stats_open(struct inode *inode, struct file *file)
    return 0;
    }

    -static int
    +static ssize_t
    minstrel_stats_read(struct file *file, char __user *buf, size_t len, loff_t *o)
    {
    struct minstrel_stats_info *ms;

    commit 04271505e03535e1fd085c96085fcee41e7c415f
    Author: Ingo Molnar
    Date: Mon Aug 18 15:31:58 2008 +0200

    [vfs] fs.h: fops_get()/fops_put(): use pointer comparison

    this warning:

    drivers/infiniband/core/uverbs_main.c: In function ‘ib_uverbs_alloc_event_file’:
    drivers/infiniband/core/uverbs_main.c:526: warning: the address of ‘uverbs_event_fops’ will always evaluate as ‘true’
    drivers/infiniband/core/uverbs_main.c:526: warning: assignment discards qualifiers from pointer target type

    triggers because we use an arithmetic comparison. Use a pointer
    comparison instead.

    No code changed:
    91bef63ad2e661e7adb4456b944cec7d uverbs_main.o.before.asm
    91bef63ad2e661e7adb4456b944cec7d uverbs_main.o.after.asm

    Signed-off-by: Ingo Molnar

    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index 686d46c..114f469 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -1597,9 +1597,9 @@ void unnamed_dev_init(void);

    /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
    #define fops_get(fops) \
    - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
    + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL))
    #define fops_put(fops) \
    - do { if (fops) module_put((fops)->owner); } while(0)
    + do { if (fops != NULL) module_put((fops)->owner); } while(0)

    extern int register_filesystem(struct file_system_type *);
    extern int unregister_filesystem(struct file_system_type *);

    commit f81ee5ca545020daf0ddfff3744199b87bbd8c70
    Author: Ingo Molnar
    Date: Fri Oct 17 14:54:48 2008 +0200

    include/linux/fs.h: improve type checking of __mandatory_lock()

    this warning:

    fs/gfs2/ops_file.c: In function ‘gfs2_flock’:
    fs/gfs2/ops_file.c:722: warning: unused variable ‘ip’

    triggers because __mandatory_lock() should not be a macro in the
    !CONFIG_FILE_LOCKING case but an inline. This improves the type
    checking and also does not trigger a warning.

    Signed-off-by: Ingo Molnar

    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index a6a625b..686d46c 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -1675,7 +1675,7 @@ static inline int break_lease(struct inode *inode, unsigned int mode)
    #else /* !CONFIG_FILE_LOCKING */
    #define locks_mandatory_locked(a) ({ 0; })
    #define locks_mandatory_area(a, b, c, d, e) ({ 0; })
    -#define __mandatory_lock(a) ({ 0; })
    +static inline int __mandatory_lock(struct inode *ino) { return 0; }
    #define mandatory_lock(a) ({ 0; })
    #define locks_verify_locked(a) ({ 0; })
    #define locks_verify_truncate(a, b, c) ({ 0; })

    commit 5baf34fc63c31ef676e0a764415b10e5cc1c7a4b
    Author: Ingo Molnar
    Date: Fri Oct 17 14:25:36 2008 +0200

    net/netlabel/netlabel_addrlist.c: add type checking to audit_log_format()

    this warning:

    net/netlabel/netlabel_addrlist.c: In function ‘netlbl_af4list_audit_addr’:
    net/netlabel/netlabel_addrlist.c:335: warning: unused variable ‘dir’
    net/netlabel/netlabel_addrlist.c: In function ‘netlbl_af6list_audit_addr’:
    net/netlabel/netlabel_addrlist.c:369: warning: unused variable ‘dir’

    is caused because audit_log_format() is a macro, hence in the
    !CONFIG_AUDIT case the compiler does not know that the variables are
    used.

    Convert it to a proper inline instead. This improves type checking
    in the !CONFIG_AUDIT case.

    Signed-off-by: Ingo Molnar

    diff --git a/include/linux/audit.h b/include/linux/audit.h
    index 6272a39..a3f78d0 100644
    --- a/include/linux/audit.h
    +++ b/include/linux/audit.h
    @@ -580,7 +580,8 @@ extern int audit_enabled;
    #define audit_log(c,g,t,f,...) do { ; } while (0)
    #define audit_log_start(c,g,t) ({ NULL; })
    #define audit_log_vformat(b,f,a) do { ; } while (0)
    -#define audit_log_format(b,f,...) do { ; } while (0)
    +static inline void __attribute__ ((format(printf, 2, 3)))
    +audit_log_format(struct audit_buffer *ab, const char *fmt, ...) { }
    #define audit_log_end(b) do { ; } while (0)
    #define audit_log_n_hex(a,b,l) do { ; } while (0)
    #define audit_log_n_string(a,c,l) do { ; } while (0)

    commit d58cbf52cb25e215c0e34048bda8ec481bdce4af
    Author: Ingo Molnar
    Date: Fri Oct 17 14:18:31 2008 +0200

    fs/afs/dir.c: fix uninitialized variable use

    this warning:

    fs/afs/dir.c: In function ‘afs_d_revalidate’:
    fs/afs/dir.c:566: warning: ‘fid.vnode’ may be used uninitialized in this function
    fs/afs/dir.c:566: warning: ‘fid.unique’ may be used uninitialized in this function

    shows us a real bug: afs_dir_get_page() could use the uninitialized
    fid variable if CONFIG_AFS_DEBUG=y to print messages, and leak kernel
    stack data to the console.

    Signed-off-by: Ingo Molnar

    diff --git a/fs/afs/dir.c b/fs/afs/dir.c
    index dfda03d..254c567 100644
    --- a/fs/afs/dir.c
    +++ b/fs/afs/dir.c
    @@ -563,7 +563,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
    static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
    {
    struct afs_vnode *vnode, *dir;
    - struct afs_fid fid;
    + struct afs_fid fid = { 0, };
    struct dentry *parent;
    struct key *key;
    void *dir_version;

    commit 19be5d76a014ce0e743848a42cbb3b579c0ad8d7
    Author: Ingo Molnar
    Date: Fri Oct 17 13:40:00 2008 +0200

    drivers/video/cirrusfb.c: remove unused variables

    fix these warnings:

    drivers/video/cirrusfb.c: In function ‘cirrusfb_setup’:
    drivers/video/cirrusfb.c:2466: warning: unused variable ‘i’
    drivers/video/cirrusfb.c:2465: warning: unused variable ‘s’

    These two parameters are not used anymore, their use got removed
    in this commit:

    a1d35a7: cirrusfb: use modedb and add mode_option parameter

    Signed-off-by: Ingo Molnar

    diff --git a/drivers/video/cirrusfb.c b/drivers/video/cirrusfb.c
    index 048b139..ace4001 100644
    --- a/drivers/video/cirrusfb.c
    +++ b/drivers/video/cirrusfb.c
    @@ -2462,8 +2462,7 @@ static int __init cirrusfb_init(void)

    #ifndef MODULE
    static int __init cirrusfb_setup(char *options) {
    - char *this_opt, s[32];
    - int i;
    + char *this_opt;

    DPRINTK("ENTER\n");


    commit d138e44e2f8d0f26e04b0386d328d9cc7e33d82e
    Author: Ingo Molnar
    Date: Fri Oct 17 14:30:37 2008 +0200

    drivers/net/wireless/b43/phy_g.c: type check debug printouts

    this warning:

    drivers/net/wireless/b43/phy_g.c: In function ‘b43_gphy_op_recalc_txpower’:
    drivers/net/wireless/b43/phy_g.c:3191: warning: unused variable ‘dbm’

    is caused because b43dbg() is a macro, hence in the !B43_DEBUG
    case the compiler does not know that the variables are used.

    Convert it to a proper inline instead. This also improves type checking
    in the !B43_DEBUG case.

    Signed-off-by: Ingo Molnar

    diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
    index 427b820..ac55b62 100644
    --- a/drivers/net/wireless/b43/b43.h
    +++ b/drivers/net/wireless/b43/b43.h
    @@ -853,7 +853,8 @@ void b43warn(struct b43_wl *wl, const char *fmt, ...)
    void b43dbg(struct b43_wl *wl, const char *fmt, ...)
    __attribute__ ((format(printf, 2, 3)));
    #else /* DEBUG */
    -# define b43dbg(wl, fmt...) do { /* nothing */ } while (0)
    +static inline void __attribute__ ((format(printf, 2, 3)))
    +b43dbg(struct b43_wl *wl, const char *fmt, ...) { }
    #endif /* DEBUG */

    /* A WARN_ON variant that vanishes when b43 debugging is disabled.

    commit f11adf3c65aff25074d5d3a90d72cdfc40d66b50
    Author: Ingo Molnar
    Date: Fri Oct 17 12:54:56 2008 +0200

    drivers/ide/pci/hpt366.c: remove unused variable

    this warning:

    drivers/ide/pci/hpt366.c: In function ‘init_hwif_hpt366’:
    drivers/ide/pci/hpt366.c:1292: warning: unused variable ‘dev’

    triggers because this variable is not used in this function at all.

    Remov it.

    Signed-off-by: Ingo Molnar

    diff --git a/drivers/ide/pci/hpt366.c b/drivers/ide/pci/hpt366.c
    index 9cf171c..ca0acb5 100644
    --- a/drivers/ide/pci/hpt366.c
    +++ b/drivers/ide/pci/hpt366.c
    @@ -1289,7 +1289,6 @@ static u8 hpt3xx_cable_detect(ide_hwif_t *hwif)

    static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
    {
    - struct pci_dev *dev = to_pci_dev(hwif->dev);
    struct hpt_info *info = hpt3xx_get_info(hwif->dev);
    int serialize = HPT_SERIALIZE_IO;
    u8 chip_type = info->chip_type;

    commit ecd05381e26b9a61e49fa485baea1595bd3d1b40
    Author: Ingo Molnar
    Date: Fri Oct 17 16:09:57 2008 +0200

    x86: fix default_spin_lock_flags() prototype

    these warnings:

    arch/x86/kernel/paravirt-spinlocks.c: In function ‘default_spin_lock_flags’:
    arch/x86/kernel/paravirt-spinlocks.c:12: warning: passing argument 1 of ‘__raw_spin_lock’ from incompatible pointer type
    arch/x86/kernel/paravirt-spinlocks.c: At top level:
    arch/x86/kernel/paravirt-spinlocks.c:11: warning: ‘default_spin_lock_flags’ defined but not used

    showed that the prototype of default_spin_lock_flags() was confused about
    what type spinlocks have.

    the proper type on UP is raw_spinlock_t.

    Signed-off-by: Ingo Molnar

    diff --git a/arch/x86/kernel/paravirt-spinlocks.c b/arch/x86/kernel/paravirt-spinlocks.c
    index 0e9f198..95777b0 100644
    --- a/arch/x86/kernel/paravirt-spinlocks.c
    +++ b/arch/x86/kernel/paravirt-spinlocks.c
    @@ -7,7 +7,8 @@

    #include

    -static void default_spin_lock_flags(struct raw_spinlock *lock, unsigned long flags)
    +static inline void
    +default_spin_lock_flags(raw_spinlock_t *lock, unsigned long flags)
    {
    __raw_spin_lock(lock);
    }
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [announce] new tree: "fix all build warnings, on all configs"

    On Mon, 20 October 2008 21:21:10 +0200, Ingo Molnar wrote:
    >
    > /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
    > #define fops_get(fops) \
    > - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
    > + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL))
    > #define fops_put(fops) \
    > - do { if (fops) module_put((fops)->owner); } while(0)
    > + do { if (fops != NULL) module_put((fops)->owner); } while(0)


    This, I would argue, makes the code worse.

    > diff --git a/include/linux/audit.h b/include/linux/audit.h
    > index 6272a39..a3f78d0 100644
    > --- a/include/linux/audit.h
    > +++ b/include/linux/audit.h
    > @@ -580,7 +580,8 @@ extern int audit_enabled;
    > #define audit_log(c,g,t,f,...) do { ; } while (0)
    > #define audit_log_start(c,g,t) ({ NULL; })
    > #define audit_log_vformat(b,f,a) do { ; } while (0)
    > -#define audit_log_format(b,f,...) do { ; } while (0)
    > +static inline void __attribute__ ((format(printf, 2, 3)))
    > +audit_log_format(struct audit_buffer *ab, const char *fmt, ...) { }


    Took me a moment to notice the two lines aren't independent. A tab
    would have been appreciated.

    > diff --git a/drivers/net/wireless/b43/b43.h b/drivers/net/wireless/b43/b43.h
    > index 427b820..ac55b62 100644
    > --- a/drivers/net/wireless/b43/b43.h
    > +++ b/drivers/net/wireless/b43/b43.h
    > @@ -853,7 +853,8 @@ void b43warn(struct b43_wl *wl, const char *fmt, ...)
    > void b43dbg(struct b43_wl *wl, const char *fmt, ...)
    > __attribute__ ((format(printf, 2, 3)));
    > #else /* DEBUG */
    > -# define b43dbg(wl, fmt...) do { /* nothing */ } while (0)
    > +static inline void __attribute__ ((format(printf, 2, 3)))
    > +b43dbg(struct b43_wl *wl, const char *fmt, ...) { }


    Dito.

    Jörn

    --
    A victorious army first wins and then seeks battle.
    -- Sun Tzu
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [announce] new tree: "fix all build warnings, on all configs" II

    Andi Kleen writes:
    >> if (battery->have_sysfs_alarm)
    >> diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
    >> index d13194a..2276d75 100644
    >> --- a/drivers/acpi/sleep/main.c
    >> +++ b/drivers/acpi/sleep/main.c
    >> @@ -63,7 +63,7 @@ void __init acpi_old_suspend_ordering(void)
    >> /**
    >> * acpi_pm_disable_gpes - Disable the GPEs.
    >> */
    >> -static int acpi_pm_disable_gpes(void)
    >> +static inline int acpi_pm_disable_gpes(void)

    >
    > Just to satisfy my curiosity, what compiler warning does marking functions inline
    > fix?


    No reply.

    General note: ignoring review comments does not make the problems go away.

    The reason I asked is that the patch is very likely wrong.

    AFAIK the only warning that can be fixed by this inline would
    be a linker section mismatch (that is why I asked).

    But for linker section mismatch this is not the correct
    change:

    - inline is only advisory and gcc is free to disregard it.
    So you could get the warning back any time.
    - If you really want inlining for correctness you need
    to use __always_inline
    - Or if it's really to satisfy a linker section mismatch
    it's typically better to just declare all inlined functions
    in the correct section, e.g. __init

    Please fix this properly.

    Thanks,
    -Andi

    --
    ak@linux.intel.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Andi Kleen wrote:

    > > * acpi_pm_disable_gpes - Disable the GPEs.
    > > */
    > > -static int acpi_pm_disable_gpes(void)
    > > +static inline int acpi_pm_disable_gpes(void)

    >
    > Just to satisfy my curiosity, what compiler warning does marking
    > functions inline fix?


    the commit log below explains the situation. The warning exposed a maze
    of #ifdefs in drivers/acpi/sleep/main.c. It's not the warning we need to
    "fix" but that maze, obviously.

    Ingo

    -------------------------------------------->
    From 6ddae344a73fcff60c840dd4e429bf55562b41f3 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Fri, 17 Oct 2008 15:44:22 +0200
    Subject: [PATCH] #ifdef complications in drivers/acpi/sleep/main.c

    this warning:

    drivers/acpi/sleep/main.c:67: warning: ‘acpi_pm_disable_gpes’ defined but not used
    drivers/acpi/sleep/main.c:92: warning: ‘acpi_pm_prepare’ defined but not used
    drivers/acpi/sleep/main.c:107: warning: ‘acpi_pm_finish’ defined but not used
    drivers/acpi/sleep/main.c:128: warning: ‘acpi_pm_end’ defined but not used

    Shows that this code has an identity crisis due to a maze of #ifdefs, in
    the PM_SLEEP && !SUSPEND && !HIBERNATION case for example.

    Instead of complicating the code with even more #ifdefs, one option
    would be to convert these rather trivial wrappers to inline functions
    (implemented in this commit). Maybe there's a better solution as well -
    such as to reduce the many config options we have in this area?

    No size difference with SUSPEND && HIBERNATION turned back on:

    drivers/acpi/sleep/main.o:

    text data bss dec hex filename
    2717 976 33 3726 e8e main.o.before
    2717 976 33 3726 e8e main.o.after

    md5:
    44220906eff0ea6e1a3266b35dd82ac2 main.o.before.asm
    44220906eff0ea6e1a3266b35dd82ac2 main.o.after.asm
    ---
    drivers/acpi/sleep/main.c | 10 +++++-----
    1 files changed, 5 insertions(+), 5 deletions(-)

    diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
    index d13194a..2276d75 100644
    --- a/drivers/acpi/sleep/main.c
    +++ b/drivers/acpi/sleep/main.c
    @@ -63,7 +63,7 @@ void __init acpi_old_suspend_ordering(void)
    /**
    * acpi_pm_disable_gpes - Disable the GPEs.
    */
    -static int acpi_pm_disable_gpes(void)
    +static inline int acpi_pm_disable_gpes(void)
    {
    acpi_hw_disable_all_gpes();
    return 0;
    @@ -75,7 +75,7 @@ static int acpi_pm_disable_gpes(void)
    * If necessary, set the firmware waking vector and do arch-specific
    * nastiness to get the wakeup code to the waking vector.
    */
    -static int __acpi_pm_prepare(void)
    +static inline int __acpi_pm_prepare(void)
    {
    int error = acpi_sleep_prepare(acpi_target_sleep_state);

    @@ -88,7 +88,7 @@ static int __acpi_pm_prepare(void)
    * acpi_pm_prepare - Prepare the platform to enter the target sleep
    * state and disable the GPEs.
    */
    -static int acpi_pm_prepare(void)
    +static inline int acpi_pm_prepare(void)
    {
    int error = __acpi_pm_prepare();

    @@ -103,7 +103,7 @@ static int acpi_pm_prepare(void)
    * This is called after we wake back up (or if entering the sleep state
    * failed).
    */
    -static void acpi_pm_finish(void)
    +static inline void acpi_pm_finish(void)
    {
    u32 acpi_state = acpi_target_sleep_state;

    @@ -124,7 +124,7 @@ static void acpi_pm_finish(void)
    /**
    * acpi_pm_end - Finish up suspend sequence.
    */
    -static void acpi_pm_end(void)
    +static inline void acpi_pm_end(void)
    {
    /*
    * This is necessary in case acpi_pm_finish() is not called during a
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [announce] new tree: "fix all build warnings, on all configs"

    On Tue, Oct 21, 2008 at 01:17:16PM +0200, Ingo Molnar wrote:
    >
    > * Andi Kleen wrote:
    >
    > > > * acpi_pm_disable_gpes - Disable the GPEs.
    > > > */
    > > > -static int acpi_pm_disable_gpes(void)
    > > > +static inline int acpi_pm_disable_gpes(void)

    > >
    > > Just to satisfy my curiosity, what compiler warning does marking
    > > functions inline fix?

    >
    > the commit log below explains the situation. The warning exposed a maze
    > of #ifdefs in drivers/acpi/sleep/main.c. It's not the warning we need to
    > "fix" but that maze, obviously.


    Thanks. That makes sense,

    I also agree with you that the better alternative would be
    to just always force SUSPEND together with ACPI.

    I suspect the code delta wouldn't be very large compared to the rest
    of the ACPI code.

    -Andi

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [announce] new tree: "fix all build warnings, on all configs"

    On Tuesday, 21 of October 2008, Andi Kleen wrote:
    > On Tue, Oct 21, 2008 at 01:17:16PM +0200, Ingo Molnar wrote:
    > >
    > > * Andi Kleen wrote:
    > >
    > > > > * acpi_pm_disable_gpes - Disable the GPEs.
    > > > > */
    > > > > -static int acpi_pm_disable_gpes(void)
    > > > > +static inline int acpi_pm_disable_gpes(void)
    > > >
    > > > Just to satisfy my curiosity, what compiler warning does marking
    > > > functions inline fix?

    > >
    > > the commit log below explains the situation. The warning exposed a maze
    > > of #ifdefs in drivers/acpi/sleep/main.c. It's not the warning we need to
    > > "fix" but that maze, obviously.

    >
    > Thanks. That makes sense,
    >
    > I also agree with you that the better alternative would be
    > to just always force SUSPEND together with ACPI.
    >
    > I suspect the code delta wouldn't be very large compared to the rest
    > of the ACPI code.


    Is that _really_ necessary?

    I mean, do the warnings appear in any case that's not theoretically impossible?

    Rafael
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  18. Re: [announce] new tree: "fix all build warnings, on all configs"

    On Tuesday, 21 of October 2008, Ingo Molnar wrote:
    >
    > * Andi Kleen wrote:
    >
    > > > * acpi_pm_disable_gpes - Disable the GPEs.
    > > > */
    > > > -static int acpi_pm_disable_gpes(void)
    > > > +static inline int acpi_pm_disable_gpes(void)

    > >
    > > Just to satisfy my curiosity, what compiler warning does marking
    > > functions inline fix?

    >
    > the commit log below explains the situation. The warning exposed a maze
    > of #ifdefs in drivers/acpi/sleep/main.c. It's not the warning we need to
    > "fix" but that maze, obviously.


    Thanks a lot for _not_ CCing me. :-(

    > -------------------------------------------->
    > From 6ddae344a73fcff60c840dd4e429bf55562b41f3 Mon Sep 17 00:00:00 2001
    > From: Ingo Molnar
    > Date: Fri, 17 Oct 2008 15:44:22 +0200
    > Subject: [PATCH] #ifdef complications in drivers/acpi/sleep/main.c
    >
    > this warning:
    >
    > drivers/acpi/sleep/main.c:67: warning: ‘acpi_pm_disable_gpes’ defined but not used
    > drivers/acpi/sleep/main.c:92: warning: ‘acpi_pm_prepare’ defined but not used
    > drivers/acpi/sleep/main.c:107: warning: ‘acpi_pm_finish’ defined but not used
    > drivers/acpi/sleep/main.c:128: warning: ‘acpi_pm_end’ defined but not used
    >
    > Shows that this code has an identity crisis due to a maze of #ifdefs, in
    > the PM_SLEEP && !SUSPEND && !HIBERNATION case for example.


    This case is invalid, because PM_SLEEP == SUSPEND || HIBERNATION !

    I don't know how you managed to get
    (PM_SLEEP && !SUSPEND && !HIBERNATION), but _that_ shouldn'd be possible in the
    first place.

    If there are warnings in any other case, please let me know and I'll fix them,
    but please don't mess up with that code like this without letting me know.

    Thanks,
    Rafael
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  19. Re: [announce] new tree: "fix all build warnings, on all configs"



    > > drivers/acpi/sleep/main.c:67: warning: ‘acpi_pm_disable_gpes’ defined but not used


    > >
    > > Shows that this code has an identity crisis due to a maze of #ifdefs, in
    > > the PM_SLEEP && !SUSPEND && !HIBERNATION case for example.

    >
    > This case is invalid, because PM_SLEEP == SUSPEND || HIBERNATION !


    I though so too, but 2.6.27 appears to have added XEN to the mix:

    config PM_SLEEP
    bool
    depends on SUSPEND || HIBERNATION || XEN_SAVE_RESTORE

    -Len


  20. Re: [announce] new tree: "fix all build warnings, on all configs"


    * Jrn Engel wrote:

    > On Mon, 20 October 2008 21:21:10 +0200, Ingo Molnar wrote:
    > >
    > > /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
    > > #define fops_get(fops) \
    > > - (((fops) && try_module_get((fops)->owner) ? (fops) : NULL))
    > > + (((fops != NULL) && try_module_get((fops)->owner) ? (fops) : NULL))
    > > #define fops_put(fops) \
    > > - do { if (fops) module_put((fops)->owner); } while(0)
    > > + do { if (fops != NULL) module_put((fops)->owner); } while(0)

    >
    > This, I would argue, makes the code worse.


    Have a look at:

    $ git log -p --grep="NULL noise"

    for example:

    for (i = 0; i < MAX_FEB_SIZE; i++)
    - if (tb->FEB[i] != 0)
    + if (tb->FEB[i] != NULL)
    break;

    so checking for != NULL is a valid way of testing a pointer's existence.
    The "if (tb->FEB[i])" is a valid shortcut for the same thing as well.

    In this specific case the issue is that the 'fops' parameter can
    occasionally be a constant pointer (turning the test into always-true)
    so the compiler is at least minimally correct at asking the "are you
    sure you want this" question - which we answer in the affirmative via
    the explicit NULL check. But these are really nuances.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 1 of 2 1 2 LastLast