* [bug report] PCI: endpoint: Automatically create a function specific attributes group
@ 2023-05-11 7:06 Dan Carpenter
2023-05-14 13:36 ` Manivannan Sadhasivam
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-05-11 7:06 UTC (permalink / raw)
To: dlemoal; +Cc: linux-pci
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?
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group
2023-05-11 7:06 [bug report] PCI: endpoint: Automatically create a function specific attributes group Dan Carpenter
@ 2023-05-14 13:36 ` Manivannan Sadhasivam
2023-05-14 20:12 ` Dan Carpenter
2023-05-15 2:14 ` Damien Le Moal
0 siblings, 2 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2023-05-14 13:36 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dlemoal, linux-pci
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.
- 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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group
2023-05-14 13:36 ` Manivannan Sadhasivam
@ 2023-05-14 20:12 ` Dan Carpenter
2023-05-15 4:22 ` Manivannan Sadhasivam
2023-05-15 2:14 ` Damien Le Moal
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-05-14 20:12 UTC (permalink / raw)
To: Manivannan Sadhasivam; +Cc: dlemoal, linux-pci
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group
2023-05-14 20:12 ` Dan Carpenter
@ 2023-05-15 4:22 ` Manivannan Sadhasivam
0 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2023-05-15 4:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dlemoal, linux-pci
On Sun, May 14, 2023 at 11:12:22PM +0300, Dan Carpenter wrote:
> 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).
>
Ah okay. I never thought Smatch would look into callback definitions. Good to
know!
- Mani
> > 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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group
2023-05-14 13:36 ` Manivannan Sadhasivam
2023-05-14 20:12 ` Dan Carpenter
@ 2023-05-15 2:14 ` Damien Le Moal
2023-05-15 4:21 ` Manivannan Sadhasivam
1 sibling, 1 reply; 6+ messages in thread
From: Damien Le Moal @ 2023-05-15 2:14 UTC (permalink / raw)
To: Manivannan Sadhasivam, Dan Carpenter; +Cc: linux-pci
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.
>
> - 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [bug report] PCI: endpoint: Automatically create a function specific attributes group
2023-05-15 2:14 ` Damien Le Moal
@ 2023-05-15 4:21 ` Manivannan Sadhasivam
0 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2023-05-15 4:21 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Dan Carpenter, linux-pci
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
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-15 4:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-11 7:06 [bug report] PCI: endpoint: Automatically create a function specific attributes group Dan Carpenter
2023-05-14 13:36 ` Manivannan Sadhasivam
2023-05-14 20:12 ` Dan Carpenter
2023-05-15 4:22 ` Manivannan Sadhasivam
2023-05-15 2:14 ` Damien Le Moal
2023-05-15 4:21 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).