Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing goto-based cleanup with scope-based cleanup helpers in the same function. --
--- Patch [9]: [PATCH V12 09/12] famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN) --- commit 92fac6008c0bb8bb7ad31e3ae10289f93997e74b Author: John Groves <[email protected]> famfs: Register secondary daxdevs by path (FAMFSIOC_DAXDEV_OPEN) This commit introduces the FAMFSIOC_DAXDEV_OPEN ioctl to register devdax devices by path into the famfs mount's daxdev table. This allows files with extents referencing these secondary devices to be mapped. > diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c > --- a/fs/famfs/famfs_file.c > +++ b/fs/famfs/famfs_file.c [ ... ] > +static int > +famfs_daxdev_open(struct file *file, void __user *arg) > +{ [ ... ] > + scoped_guard(rwsem_read, &fsi->devlist_sem) { > + if (dd.daxdev_index >= fsi->dax_devlist->nslots) > + return -EINVAL; > + if (fsi->dax_devlist->devlist[dd.daxdev_index].valid) > + return 0; > + } > + > + if (dd.daxdev_path_len == 0 || dd.daxdev_path_len >= PATH_MAX) > + return -EINVAL; > + > + /* +1 so the terminating NUL is included within the bound */ > + path = strndup_user((const char __user *)(uintptr_t)dd.daxdev_path, > + dd.daxdev_path_len + 1); > + if (IS_ERR(path)) > + return PTR_ERR(path); > + > + rc = lookup_daxdev(path, &devno); > + if (rc) > + goto out; [Severity: Low] Does famfs_daxdev_open() mix goto-based cleanup with scope-based cleanup helpers? This function uses scoped_guard() earlier, but relies on a traditional goto out label for manual memory cleanup of the path variable. The cleanup subsystem guidelines recommend against mixing goto-based error handling and scope-based cleanup in the same function to avoid confusing ownership semantics. Could path be declared with the __free() helper to eliminate the need for the goto label entirely? > + > + /* > + * The daxdev table is allocated at mount time (for the slot-0 primary), > + * so it is always present here; no need to allocate it. > + */ > + rc = famfs_install_daxdev(fsi, sb, dd.daxdev_index, devno, path); > + if (rc) > + pr_debug("%s: failed to install daxdev index %llu (%s)\n", > + __func__, dd.daxdev_index, path); > +out: > + kfree(path); > + return rc; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/0100019fc572ca94-ec363dd7-3a77-484b-b4b7-f2503a0931a6-000...@email.amazonses.com?part=9
