Hello.
On Thu, Apr 27, 2023 at 11:08:41AM +0800, Zheng Wang <zyytlz.wz@xxxxxxx> wrote:
> static int imsttfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> @@ -1529,10 +1530,10 @@ static int imsttfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (!par->cmap_regs)
> goto error;
> info->pseudo_palette = par->palette;
> - init_imstt(info);
> -
> - pci_set_drvdata(pdev, info);
> - return 0;
> + ret = init_imstt(info);
> + if (!ret)
> + pci_set_drvdata(pdev, info);
> + return ret;
>
> error:
> if (par->dc_regs)
This part caught my eye -- shouldn't the -ENODEV from init_imstt go
through the standard error with proper cleanup? (It seems like a leak
from my 30000 ft view, i.e. not sure about imsttfb_{probe,remove}
pairing.)
Shouldn't there be something like the diff below on top of the existing code?
Regards,
Michal
diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c
index 975dd682fae4..a116ac8ca020 100644
--- a/drivers/video/fbdev/imsttfb.c
+++ b/drivers/video/fbdev/imsttfb.c
@@ -1419,7 +1419,6 @@ static int init_imstt(struct fb_info *info)
if ((info->var.xres * info->var.yres) * (info->var.bits_per_pixel >> 3) > info->fix.smem_len
|| !(compute_imstt_regvals(par, info->var.xres, info->var.yres))) {
printk("imsttfb: %ux%ux%u not supported\n", info->var.xres, info->var.yres, info->var.bits_per_pixel);
- framebuffer_release(info);
return -ENODEV;
}
@@ -1455,7 +1454,6 @@ static int init_imstt(struct fb_info *info)
fb_alloc_cmap(&info->cmap, 0, 0);
if (register_framebuffer(info) < 0) {
- framebuffer_release(info);
return -ENODEV;
}
@@ -1531,8 +1529,10 @@ static int imsttfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
goto error;
info->pseudo_palette = par->palette;
ret = init_imstt(info);
- if (!ret)
- pci_set_drvdata(pdev, info);
+ if (ret)
+ goto error;
+
+ pci_set_drvdata(pdev, info);
return ret;
error:
Attachment:
signature.asc
Description: PGP signature
