Hi Lukas,
On 25/06/21 11:46AM, Lukas Wunner wrote:
> On Sat, Jun 21, 2025 at 04:55:52AM +0300, Ahmed Salem wrote:
> > --- a/drivers/char/agp/amd64-agp.c
> > +++ b/drivers/char/agp/amd64-agp.c
> > @@ -768,10 +768,15 @@ int __init agp_amd64_init(void)
> >
> > /* Look for any AGP bridge */
> > agp_amd64_pci_driver.id_table = agp_amd64_pci_promisc_table;
> > - err = driver_attach(&agp_amd64_pci_driver.driver);
> > - if (err == 0 && agp_bridges_found == 0) {
> > + if ((int *)agp_amd64_pci_driver.probe != 0) {
> > pci_unregister_driver(&agp_amd64_pci_driver);
> > err = -ENODEV;
> > + } else {
> > + err = driver_attach(&agp_amd64_pci_driver.driver);
> > + if (err == 0 && agp_bridges_found == 0) {
> > + pci_unregister_driver(&agp_amd64_pci_driver);
> > + err = -ENODEV;
> > + }
>
> Is the "probe" member in agp_amd64_pci_driver overwritten with a
> zero pointer anywhere? I don't see that it is, so it seems the
> else-branch is never entered.
>
That is a great question. I thought since pci_register_driver calls the
probe function, it would return with or without a zero, saving that
value in the .probe member. (I find my lack of C
experience...disturbing)
> I had already prepared a fix for this, but waited for 0-day to
> crunch through it. I've just submitted it, so that's what I had
> in mind:
>
> https://lore.kernel.org/r/f8ff40f35a9a5836d1371f60e85c09c5735e3c5e.1750497201.git.lu...@wunner.de/
>
That one I've seen even prior to catching this one, and this is
originally what I had in mind based on what commit 6fd024893911
("amd64-agp: Probe unknown AGP devices the right way") removed (i.e.
!pci_find_capability) when you suggested checking for caps beforehand,
but I figured "why make other calls when .probe already does it right
off the bat?" Clearly, I was in the wrong there.
Nonetheless, thank you so much for the review, and Ccing me in the above
patch. Ultimately, what matters to me is getting the right fix in order,
and learning not only what I'd missed here (i.e. Reported-by trailer;
just wasn't quite certain how to approach the situation with SDN in
mind, as well as the original Cced developers), but also fundamental C
knowledge I've been trying to acquire. I appreciate you!
--
Best regards,
Ahmed Salem <x0rw...@gmail.com>
> Thanks,
>
> Lukas
