linux-kernel - Re: [PATCH] crypto: ccp: Remove forward declaration

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Sep 2018 23:41:32 +0000
From:   Gary R Hook <ghook@....com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
CC:     Nathan Chancellor <natechancellor@...il.com>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "Hook, Gary" <Gary.Hook@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] crypto: ccp: Remove forward declaration

On 09/24/2018 04:44 PM, Nick Desaulniers wrote:
> On Mon, Sep 24, 2018 at 2:27 PM Gary R Hook <ghook@....com> wrote:
>>
>> On 09/24/2018 03:24 PM, Nick Desaulniers wrote:
>>> On Mon, Sep 24, 2018 at 12:18 PM Gary R Hook <ghook@....com> wrote:
>>>>
>>>> On 09/24/2018 12:26 PM, Nathan Chancellor wrote:
>>>>> Clang emits a warning about this construct:
>>>>>
>>>>> drivers/crypto/ccp/sp-platform.c:36:36: warning: tentative array
>>>>> definition assumed to have one element
>>>>> static const struct acpi_device_id sp_acpi_match[];
>>>>>                                       ^
>>>>> 1 warning generated.
>>>>>
>>>>> Just remove the forward declarations and move the initializations up
>>>>> so that they can be used in sp_get_of_version and sp_get_acpi_version.
>>>>
>>>> I'm not going to out and out object to this just yet.
>>>
>>> Tentative array definitions need to have the correct length specified;
>>> it should either be forward declared as the correct length, or as this
>>> patch does, skip the forward declare and move up the definition.
>>> Thanks for this patch Nathan.
>>> Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>
>>
>>
>> Checked my C99, and using both static and [] to create a declaration
> 
> Note that the kernel uses gnu89, which is ISO C90 + gnu extensions (IIUC).

I understand that (and appreciate that ubiquitousness of the dialect) 
but in this case it sounds like we are interested in the widest possible 
compatibility. Which makes the original patch problematic, and the fix 
suggested herein suitable.

> 
>> (because it's really a definition) apparently isn't valid. I should have
>> known that. Since I'm old school...
>>
>> Acked-by: Gary R Hook <gary.hook@....com>
>>
>>
>>>
>>>>
>>>> I am not a clang expert. Can you please provide a make command that
>>>> would explain how you precipitated this complaint?
>>>>
>>>>
>>>>> Reported-by: Nick Desaulniers <ndesaulniers@...gle.com>
>>>>> Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
>>>>> ---
>>>>>     drivers/crypto/ccp/sp-platform.c | 53 +++++++++++++++-----------------
>>>>>     1 file changed, 25 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/drivers/crypto/ccp/sp-platform.c b/drivers/crypto/ccp/sp-platform.c
>>>>> index 71734f254fd1..b75dc7db2d4a 100644
>>>>> --- a/drivers/crypto/ccp/sp-platform.c
>>>>> +++ b/drivers/crypto/ccp/sp-platform.c
>>>>> @@ -33,8 +33,31 @@ struct sp_platform {
>>>>>         unsigned int irq_count;
>>>>>     };
>>>>>
>>>>> -static const struct acpi_device_id sp_acpi_match[];
>>>>> -static const struct of_device_id sp_of_match[];
>>>>> +static const struct sp_dev_vdata dev_vdata[] = {
>>>>> +     {
>>>>> +             .bar = 0,
>>>>> +#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>>>> +             .ccp_vdata = &ccpv3_platform,
>>>>> +#endif
>>>>> +     },
>>>>> +};
>>>>> +
>>>>> +#ifdef CONFIG_ACPI
>>>>> +static const struct acpi_device_id sp_acpi_match[] = {
>>>>> +     { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>>>> +     { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>>>> +#endif
>>>>> +
>>>>> +#ifdef CONFIG_OF
>>>>> +static const struct of_device_id sp_of_match[] = {
>>>>> +     { .compatible = "amd,ccp-seattle-v1a",
>>>>> +       .data = (const void *)&dev_vdata[0] },
>>>>> +     { },
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, sp_of_match);
>>>>> +#endif
>>>>>
>>>>>     static struct sp_dev_vdata *sp_get_of_version(struct platform_device *pdev)
>>>>>     {
>>>>> @@ -201,32 +224,6 @@ static int sp_platform_resume(struct platform_device *pdev)
>>>>>     }
>>>>>     #endif
>>>>>
>>>>> -static const struct sp_dev_vdata dev_vdata[] = {
>>>>> -     {
>>>>> -             .bar = 0,
>>>>> -#ifdef CONFIG_CRYPTO_DEV_SP_CCP
>>>>> -             .ccp_vdata = &ccpv3_platform,
>>>>> -#endif
>>>>> -     },
>>>>> -};
>>>>> -
>>>>> -#ifdef CONFIG_ACPI
>>>>> -static const struct acpi_device_id sp_acpi_match[] = {
>>>>> -     { "AMDI0C00", (kernel_ulong_t)&dev_vdata[0] },
>>>>> -     { },
>>>>> -};
>>>>> -MODULE_DEVICE_TABLE(acpi, sp_acpi_match);
>>>>> -#endif
>>>>> -
>>>>> -#ifdef CONFIG_OF
>>>>> -static const struct of_device_id sp_of_match[] = {
>>>>> -     { .compatible = "amd,ccp-seattle-v1a",
>>>>> -       .data = (const void *)&dev_vdata[0] },
>>>>> -     { },
>>>>> -};
>>>>> -MODULE_DEVICE_TABLE(of, sp_of_match);
>>>>> -#endif
>>>>> -
>>>>>     static struct platform_driver sp_platform_driver = {
>>>>>         .driver = {
>>>>>                 .name = "ccp",
>>>>>
>>>>
>>>
>>>
>>
> 
>