Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The patch adds redundant cleanup code based on a false premise, violating the Nouveau object lifecycle design. --
commit bc4fa3965bb025027291bc92ec2fa2ec144efd31 Author: WenTao Liang <[email protected]> fix: drm/nouveau: nvkm_ucgrp_new: nvkm_cgrp_new failure leaks ucgrp object and engine reference This commit attempts to fix a purported resource leak in nvkm_ucgrp_new() by adding an explicit call to nvkm_object_del() when nvkm_cgrp_new() fails. > The caller ignores *pobject on error, so the ucgrp object and its engine > reference are permanently leaked. Is this description of the caller's behavior accurate? The core framework caller, nvkm_ioctl_new(), appears to correctly capture the object pointer and invoke nvkm_object_del() when the constructor returns an error code. Since *pobject is assigned before the failure in nvkm_ucgrp_new(), does a leak actually occur here? > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ucgrp.c > b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ucgrp.c > index dfa3c7dbdf348..7d11b92f946ba 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ucgrp.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ucgrp.c > @@ -113,8 +113,10 @@ nvkm_ucgrp_new(struct nvkm_fifo *fifo, const struct > nvkm_oclass *oclass, void *a > *pobject = &ucgrp->object; > > ret = nvkm_cgrp_new(runl, args->v0.name, vmm, true, &ucgrp->cgrp); > - if (ret) > + if (ret) { > + nvkm_object_del(pobject); [Severity: Low] Does adding this explicit cleanup conflict with the subsystem's centralized object lifecycle design? Because the framework caller handles the cleanup of partially constructed objects when an error is returned, is this manual call to nvkm_object_del() redundant? > goto done; > + } > > /* Return channel group info to caller. */ > args->v0.cgid = ucgrp->cgrp->id; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
