linux-kernel - Re: [PATCH V6 4/7] OF: properties: Implement get_match

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]

Date:   Fri, 8 Dec 2017 09:40:02 -0500
From:   Sinan Kaya <okaya@...eaurora.org>
To:     Lothar Waßmann <LW@...O-electronics.de>
Cc:     devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        timur@...eaurora.org, open list <linux-kernel@...r.kernel.org>,
        linux-acpi@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        dmaengine@...r.kernel.org, Frank Rowand <frowand.list@...il.com>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH V6 4/7] OF: properties: Implement get_match_data()
 callback

On 12/8/2017 9:33 AM, Sinan Kaya wrote:
> On 12/8/2017 4:11 AM, Lothar Waßmann wrote:
>>> While implementing the ACPI piece, I have to convert an unsigned long to
>>> (const void *) in ACPI code so that the APIs are compatible.
>>>
>> Just one more remark: Do you need write access to the data the pointer
>> returned by device_get_match_data() or of_fwnode_get_match_data()
>> points to?
>> If not, the return type of those functions should be 'const void *'.
> 
> Yes, the only reason driver is trying to obtain this data pointer is to modify
> members of it. I did a quick test with your suggestion.

I guess I should soften my statement here. I look at examples, they seem to be read.
Even for my HIDMA case:

static const struct of_device_id hidma_match[] = {
	{.compatible = "qcom,hidma-1.0",},
	{.compatible = "qcom,hidma-1.1", .data = (void *)(HIDMA_MSI_CAP),},
	{.compatible = "qcom,hidma-1.2",
	 .data = (void *)(HIDMA_MSI_CAP | HIDMA_IDENTITY_CAP),},
	{},
}; 

I can change the return type to (const void*) if Rob, Rafael and Sakari agrees.
I didn't really like converting a long to const void*. That's my personal opinion.

> 
> test.c: In function ‘main’:
> test.c:15:2: error: assignment of member ‘m’ in read-only object
>   t->m = 3;
>   ^
> 
> struct test
> {
>         int m;
> };
> 
> int main(void)
> {
>         const void *ptr;
>         unsigned long l =4;
>         const struct test *t;
> 
>         ptr = (const void *)l;
> 
>         t = ptr;
>         t->m = 3;
> }
> 
> 
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.