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

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, May 15, 2023 at 11:14:07AM +0900, Damien Le Moal wrote:
> On 5/14/23 22:36, 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.
> > 
> > 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.
> 
> What do you mean with "It already does for NULL part" ? There is no kdoc for
> pci_epf_type_add_cfs() that I can see in pci/next. But I am preparing one patch
> to add that. Sending soon.
> 

Heh, I was looking at 6.4-rc1 and in -next, your patch removed the kdoc while
moving the function to pci-ep-cfs.c :/

So yeah, please add it back with ERR_PTR info.

- Mani

> > 
> > - Mani
> > 
> >>     541                 dev_err(&epf_group->epf->dev,
> >>     542                         "failed to create epf type specific attributes\n");
> >>     543                 return;
> >>     544         }
> >>     545 
> >>     546         configfs_register_group(&epf_group->group, group);
> >>     547 }
> >>
> >> regards,
> >> dan carpenter
> > 
> 
> -- 
> Damien Le Moal
> Western Digital Research
> 

-- 
மணிவண்ணன் சதாசிவம்