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