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.
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.
>
> 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
