On Fri, Nov 08, 2019 at 11:16:47AM +0200, Jani Nikula wrote:
On Thu, 07 Nov 2019, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote:
Convert the code to return-early style and fix missing calls
to release_firmware() if vbt is not valid.
I don't understand where anything would leak in the current code. Please
elaborate.
It was my failure reading the current code... not sure why on a first
read I thought it was leaking.
You could make a case for a change in style to avoid so much
indentation, but don't claim it fixes stuff if it doesn't.
yeah. I will keep it out for now.
Thanks
Lucas De Marchi
Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
drivers/gpu/drm/i915/display/intel_opregion.c | 28 +++++++++++--------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index 969ade623691..9738511147b1 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -872,23 +872,29 @@ static int intel_load_vbt_firmware(struct drm_i915_private *dev_priv)
return ret;
}
- if (intel_bios_is_valid_vbt(fw->data, fw->size)) {
- opregion->vbt_firmware = kmemdup(fw->data, fw->size, GFP_KERNEL);
- if (opregion->vbt_firmware) {
- DRM_DEBUG_KMS("Found valid VBT firmware \"%s\"\n", name);
- opregion->vbt = opregion->vbt_firmware;
- opregion->vbt_size = fw->size;
- ret = 0;
- } else {
- ret = -ENOMEM;
- }
- } else {
+ if (!intel_bios_is_valid_vbt(fw->data, fw->size)) {
DRM_DEBUG_KMS("Invalid VBT firmware \"%s\"\n", name);
ret = -EINVAL;
+ goto err_release_fw;
+ }
+
+ opregion->vbt_firmware = kmemdup(fw->data, fw->size, GFP_KERNEL);
+ if (!opregion->vbt_firmware) {
+ ret = -ENOMEM;
+ goto err_release_fw;
}
+ opregion->vbt = opregion->vbt_firmware;
+ opregion->vbt_size = fw->size;
+
+ DRM_DEBUG_KMS("Found valid VBT firmware \"%s\"\n", name);
+
release_firmware(fw);
+ return 0;
With ret = 0 at the beginning you could just remove the the above three
lines and let this run through the below code.
+
+err_release_fw:
+ release_firmware(fw);
Usually we'd have a blank line before the ret.
BR,
Jani.
return ret;
}
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
