From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jani Nikula <jani.nikula@linux.intel.com>
Subject: Re: [PATCH 3/9] drm/i915: fix memleak in
	intel_set_config_save_state()
Date: Fri, 29 Aug 2014 13:00:43 +0300
Message-ID: <87ha0vipis.fsf@intel.com>
References: <1409247613-14232-1-git-send-email-gustavo@padovan.org>
 <1409247613-14232-3-git-send-email-gustavo@padovan.org>
 <87k35riw3g.fsf@intel.com> <20140829075020.GD3557@nuc-i3427.alporthouse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces@lists.freedesktop.org>
In-Reply-To: <20140829075020.GD3557@nuc-i3427.alporthouse.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
 <mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org, Gustavo Padovan <gustavo.padovan@collabora.co.uk>, dri-devel@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, 29 Aug 2014, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Aug 29, 2014 at 10:38:43AM +0300, Jani Nikula wrote:
>> On Thu, 28 Aug 2014, Gustavo Padovan <gustavo@padovan.org> wrote:
>> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> >
>> > If the save_encoder_crtcs or save_connector_encoders allocation fail
>> > we need to free everything we have already allocated.
>> 
>> There is no memleak, because the caller of intel_set_config_save_state()
>> checks the return value, and subsequently handles the error with a call
>> to intel_set_config_free(), which does the right thing.
>> 
>> I know one could argue this should be done within
>> intel_set_config_save_state() but I'm not convinced. I'd let this be as
>> it is.
>> 
>> Just in case Daniel disagrees with me here and wants to merge, the patch
>> does look correct. So r-b for correctness, but nak on merging from
>> me. ;)
>
> You just said it triggers a double free... And you would be right.

Thanks Chris. I'll retract any hint of r-b I was giving. NAK.

BR,
Jani.


> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center