On Sun, Jul 20, 2025 at 10:05:43AM +0200, Eli Billauer wrote:
Hello,
On 19/07/2025 15:17, 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.
Sorry, to the extent it matters, I'm not acknowledging this.
This is merely a code styling issue, and as far as I know, there is no "standard kernel practice" on this matter. If such standard practice exists, the correct way is to prepare a patchset of *all* occurrences of kobject_put() used this way, and replace them all (e.g. fs/char_dev.c, uio/uio.c and tty/tty_io.c). Should xillybus_class be included in this patchset, I will of course ack it, not that it would matter much.
In my opinion, using cdev_del() is incorrect, as you can't delete something that failed to be added. Practically this causes no problem, but as a question of style, the kobject_put() call acts as the reversal of cdev_alloc(). This is formally more accurate, and this is the reason I chose to do it this way.
But the "problem" is that just the kobject_put() might not be enough, the kmap needs to be freed. But the map might be the part that failed, so then the kobject_put() is incorrect to be calling!
So yes, I agree, this patch shouldn't be applied, it's not going to solve the real problem here in that the cdev api is a total mess in many places like this (full disclosure, it's probably all my fault as well...)
As failing this codepath is NOT triggerable by a user, this whole discussion is just theory at this point in time. So let's just leave it alone.
Long-term, the cdev api should be fixed up, it's been on my TODO list for decades now, and in a way I'm punting it to just wait for the rust versions of drivers to take over as that's the correct fix for many of these types of corner cases as the bindings will get this right.
Ma Ke, if you want a real project, please work on fixing up the cdev api, don't worry about code paths that never can be hit like this.
thanks,
greg k-h