On 18/07/2025 12:08, Ma Ke wrote:
Use cdev_del() instead of direct kobject_put() when cdev_add() fails. This aligns with standard kernel practice and maintains consistency within the driver's own error paths.
Could you please point at how and why this is "standard kernel practice"? In my reply to PATCH v2, I pointed out that indeed, in fs/fuse/cuse.c a failure of cdev_add() leads to a call to cdev_del(), like you suggested. However, in uio/uio.c the same scenario is handled by a call to kobject_put(), exactly as in my driver. So which way is "standard"?
There are indeed kernel-global efforts to align code with a certain coding style every now and then. Is there any such in relation to this issue?
Otherwise, please leave this alone. Playing around with error handling flows is a dangerous business, and can lead to vulnerabilities. One needs a good reason to do that on code that has been out there for a while (four years, in this case).
And now, to the patch itself:
@@ -157,8 +156,6 @@ int xillybus_init_chrdev(struct device *dev, device_destroy(&xillybus_class, MKDEV(unit->major, i + unit->lowest_minor));
- cdev_del(unit->cdev);
- unregister_chrdev: unregister_chrdev_region(MKDEV(unit->major, unit->lowest_minor), unit->num_nodes);
Why did you do this? It just adds a memory leak, and it's out of the way of even trying to fix anything: The only effect this has is that cdev_del() isn't called on error situations that occur after cdev_add() has been successful. It has nothing to do with the kobject_put() / cdev_add() thing, because that case jumps to unregister_chrdev, which is after this removal.
I have to say, both the language of the patch description as well as the weird removal of cdev_del() remind me of nonsense ChatGPT does when it's given tasks related to programming. If you're using AI to suggest patches, please stop.
Regards, Eli