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.
> I'd actually be in favor of making the string a more strict UAPI than
> allowing it to be empty (at least, if heap name strings is the API we
> decide on for identifying heaps - which is another discussion).
>
I would think identifying heaps by name would be less portable, but as
you said that is a whole different discussion..
Thanks,
Andrew
> Cheers,
> -Brian
>
>>
>> Signed-off-by: Andrew F. Davis <afd@xxxxxx>
>> ---
>> drivers/staging/android/ion/ion.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
>> index bba5f682bc25..14e48f6eb734 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -467,7 +467,7 @@ static int ion_query_heaps(struct ion_heap_query *query)
>> max_cnt = query->cnt;
>>
>> plist_for_each_entry(heap, &dev->heaps, node) {
>> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> + strncpy(hdata.name, heap->name ?: "(null)", MAX_HEAP_NAME);
>> hdata.name[sizeof(hdata.name) - 1] = '\0';
>> hdata.type = heap->type;
>> hdata.heap_id = heap->id;
>> --
>> 2.19.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
