Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, May 14, 2023 at 07:06:52PM +0530, Manivannan Sadhasivam wrote:
> On Thu, May 11, 2023 at 10:06:40AM +0300, Dan Carpenter wrote:
> > Hello Damien Le Moal,
> > 
> > The patch 01c68988addf: "PCI: endpoint: Automatically create a
> > function specific attributes group" from Apr 15, 2023, leads to the
> > following Smatch static checker warning:
> > 
> > 	drivers/pci/endpoint/pci-ep-cfs.c:540 pci_ep_cfs_add_type_group()
> > 	warn: 'group' isn't an ERR_PTR
> > 
> > drivers/pci/endpoint/pci-ep-cfs.c
> >     532 static void pci_ep_cfs_add_type_group(struct pci_epf_group *epf_group)
> >     533 {
> >     534         struct config_group *group;
> >     535 
> >     536         group = pci_epf_type_add_cfs(epf_group->epf, &epf_group->group);
> >     537         if (!group)
> >     538                 return;
> >     539 
> > --> 540         if (IS_ERR(group)) {
> > 
> > pci_epf_type_add_cfs() does not return error pointers currently.  Which
> > is fine.  Presumably it will start returning them later.  But could you
> > add some comments next to the pci_epf_type_add_cfs() to explain what a
> > NULL return means vs an error pointer return?
> > 
> 
> pci_epf_type_add_cfs() may return ERR_PTR from add_cfs() callback.
> 

Right, we presumably will add new implementations of ->add_cfs in the
future which return error pointers.  Currently in linux-next there are
only two and neither of them fail.  (Smatch looks at the returns from
->add_cfs when it says that these don't return error pointers.  Of
course, it depends on your .config as well.  But a quick grep confirms
that both implementations are included in my ARM allmodconfig).

> Regarding comments, it should be added as a part of kdoc for
> pci_epf_type_add_cfs(). It already does for NULL part but not for ERR_PTR.

Excelent thanks.

regards,
dan carpenter




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux