Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Mixing goto-based error handling with scope-based cleanup helpers (`scoped_guard`) in the same function. --
commit a312843839f1a20e85a81d94af26656e4dacb18f Author: Cristian Ciocaltea <[email protected]> drm/bridge: Fix unlocked list access in drm_bridge_attach() This patch fixes an unlocked read of bridge->list in drm_bridge_attach() by performing the check under bridge_lock. It uses the scoped_guard() cleanup helper to manage the lock. > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index a8b6df5c13eaf..10dde3600c70a 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -606,8 +606,10 @@ int drm_bridge_attach(struct drm_encoder *encoder, > struct drm_bridge *bridge, > if (!bridge->container) > DRM_WARN("DRM bridge corrupted or not allocated by > devm_drm_bridge_alloc()\n"); > > - if (list_empty(&bridge->list)) > - DRM_WARN("Missing drm_bridge_add() before attach\n"); > + scoped_guard(mutex, &bridge_lock) { > + if (list_empty(&bridge->list)) > + DRM_WARN("Missing drm_bridge_add() before attach\n"); > + } [Severity: Low] Does introducing scoped_guard() here conflict with the existing goto-based error handling later in drm_bridge_attach()? The kernel's cleanup.h guidelines forbid mixing goto-based cleanup and scope-based cleanup helpers in the same function. drm_bridge_attach() currently uses goto labels for resource unwinding: if (bridge->funcs->attach) { ret = bridge->funcs->attach(bridge, encoder, flags); if (ret < 0) goto err_reset_bridge; } While this specific change doesn't cause a direct runtime leak since the goto statements don't jump into or out of the scoped_guard block scope, could this structural mixing cause issues later? Would it be safer to use standard locking (mutex_lock and mutex_unlock) here to avoid mixing the two paradigms? > > drm_bridge_get(bridge); -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
