linux-kernel - Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGngYiXBdT1jOwRS_A03iQiGRbcpPncBxtVOsnFpmauO4ffS5A@mail.gmail.com>
Date:   Sat, 11 Jul 2020 11:08:24 -0400
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     Mark Brown <broonie@...nel.org>, kernel test robot <lkp@...el.com>,
        kbuild-all@...ts.01.org, clang-built-linux@...glegroups.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: sound/soc/codecs/zl38060.c:614:34: warning: unused variable 'zl38_dt_ids'

Hi Nathan and Mark,

On Fri, Jul 10, 2020 at 11:30 PM Nathan Chancellor
<natechancellor@...il.com> wrote:
>
> Sure, you could hide it behind an ifdef for either CONFIG_OF or MODULE
> (since you could build this as a module with CONFIG_OF disabled).
>
> I just figured this would be something frowned upon but if that is how
> you would prefer it to be fixed, then I have no objections to it.
>

That's how things used to work in the past: we had to #ifdef CONFIG_OF
all the driver of_XXX code, so it could be built on !CONFIG_OF.

I remember problems with this approach: it generated a lot of
visual noise, it was fragile in both directions: including too much,
and excluding too much. Last but not least, it required us ARM people
to test build each patch on !CONFIG_OF, which many conveniently forgot
to do :)

My "vote" would be to fix the warning using compiler magic. For
example:

#if !CONFIG_OF
// #define of_match_ptr(x) NULL
#define of_match_ptr(x) ((x) == NULL ? NULL : NULL)
#endif

That seems to eliminate the warning on my gcc version 7.5.0, but of
course as compilers get more clever, this could eventually
generate a warning (if it doesn't already on clang).

So perhaps use compiler attributes instead?

Stand-alone testable code snippet below.
======================================================

// gcc -Wall unused.c
// results in: match_table is NULL
// gcc -Wall -DCONFIG_OF unused.c
// results in: match_table[0] = 5

#include <stdio.h>

#ifdef CONFIG_OF
#define of_match_ptr(x) x
#else
#define of_match_ptr(x) ((x) == NULL ? NULL : NULL)
#endif

struct of_device_id {
    int id;
};

static const struct of_device_id some_ids[] = {
    { .id = 5, },
    { /* sentinel */ }
};

struct driver {
    const struct of_device_id *of_match_table;
};

static const struct driver my_driver = {
    .of_match_table = of_match_ptr(some_ids),
};

int main()
{
    if (my_driver.of_match_table)
        printf("match_table[0] = %d\n", my_driver.of_match_table[0].id);
    else
        printf("match_table is NULL\n");
}