[lkml]   [2021]   [Feb]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v4 15/15] dmaengine: dw-edma: Add pcim_iomap_table return checker
Hi Gustavo,

> > This "pcim_iomap_table(dev)[n]" pattern is extremely common. There
> > are over 100 calls of pcim_iomap_table(), and
> >
> > $ git grep "pcim_iomap_table(.*)\[.*\]" | wc -l
> >
> > says about 75 of them are of this form, where we dereference the
> > result before testing it.
> That's true, there are a lot of drivers that don't verify that pointer.
> What do you suggest?
> 1) To remove the verification so that is aligned with the other drivers
> 2) Leave it as is. Or even to add this verification to the other drivers?
> Either way, I will add the pcim_iomap_table(pdev) before this
> instruction.

A lot of the drivers consume the value from pcim_iomap_table() at
a given BAR index directly as-is, some check if the pointer they got
back is not NULL, a very few also check if the address at a given index
is not NULL.

Given that the memory allocation for the table can fail, we ought to
check for a NULL pointer. It's a bit worrying that people decided to
consume the value it returns directly without any verification.

I only found two drivers that perform this additional verification of
checking whether the address at a given index is valid, as per:

Personally, I would opt for (2), and then like you suggested send
a separate series to update other drivers so that they also include the
this NULL pointer check.

But let's wait for Bjorn's take on this, though.


 \ /
  Last update: 2021-02-09 20:46    [W:0.079 / U:3.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site