Re: [PATCH 11/14] staging: android: ion: Allow heap name to be null

From: Andrew F. Davis
Date: Mon Jan 21 2019 - 09:30:52 EST


On 1/18/19 1:53 PM, Laura Abbott wrote:
> On 1/16/19 9:12 AM, Andrew F. Davis wrote:
>> On 1/16/19 9:28 AM, Brian Starkey wrote:
>>> Hi Andrew,
>>>
>>> On Fri, Jan 11, 2019 at 12:05:20PM -0600, Andrew F. Davis wrote:
>>>> The heap name can be used for debugging but otherwise does not seem
>>>> to be required and no other part of the code will fail if left NULL
>>>> except here. We can make it required and check for it at some point,
>>>> for now lets just prevent this from causing a NULL pointer exception.
>>>
>>> I'm not so keen on this one. In the "new" API with heap querying, the
>>> name string is the only way to identify the heap. I think Laura
>>> mentioned at XDC2017 that it was expected that userspace should use
>>> the strings to find the heap they want.
>>>
>>
>> Right now the names are only for debug. I accidentally left the name
>> null once and got a kernel crash. This is the only spot where it is
>> needed so I fixed it up. The other option is to make the name mandatory
>> and properly error out, I don't want to do that right now until the
>> below discussion is had to see if names really do matter or not.
>>
>
> Yes, the heap names are part of the query API and are the expected
> way to identify individual heaps for the API at the moment so having
> a null heap name is incorrect. The heap name seemed like the best way
> to identify heaps to userspace but if you have an alternative proposal
> I'd be interested.
>

Not sure I have a better proposal right now, I'll re-work this patch to
force heap names to be populated before ion_device_add_heap() instead.

(do you think that function name is now is a misnomer? how do you feel
about renaming that to just ion_add_heap()?)

Andrew

> Thanks,
> Laura
>
>>
>
>