Return-Path: Date: Mon, 23 Jan 2012 13:12:05 +0200 From: Johan Hedberg To: Vinicius Costa Gomes Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH BlueZ v2 3/4] Fix memory leak when loading keys Message-ID: <20120123111205.GE13100@x220> References: <1326928412-25910-1-git-send-email-vinicius.gomes@openbossa.org> <1326928412-25910-3-git-send-email-vinicius.gomes@openbossa.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1326928412-25910-3-git-send-email-vinicius.gomes@openbossa.org> Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Vinicius, On Wed, Jan 18, 2012, Vinicius Costa Gomes wrote: > If we need a copy of those keys we should copy them. > --- > plugins/hciops.c | 13 ++++++++++++- > 1 files changed, 12 insertions(+), 1 deletions(-) The two first patches have been applied but this one had some issues: > diff --git a/plugins/hciops.c b/plugins/hciops.c > index d4d219c..4e0729e 100644 > --- a/plugins/hciops.c > +++ b/plugins/hciops.c > @@ -3563,6 +3563,7 @@ static int hciops_restore_powered(int index) > static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys) > { > struct dev_info *dev = &devs[index]; > + GSList *l, *new; > > DBG("hci%d keys %d debug_keys %d", index, g_slist_length(keys), > debug_keys); > @@ -3570,7 +3571,17 @@ static int hciops_load_keys(int index, GSList *keys, gboolean debug_keys) > if (dev->keys != NULL) > return -EEXIST; > > - dev->keys = keys; > + for (new = NULL, l = keys; l; l = l->next) { > + struct link_key_info *orig, *dup; > + > + orig = l->data; > + > + dup = g_memdup(orig, sizeof(*orig)); > + > + new = g_slist_prepend(new, dup); > + } > + > + dev->keys = new; > dev->debug_keys = debug_keys; In general I'm always a bit on guard with usage of C++ keywords like new and class, but in this case instead of renaming you could just prepend directly to dev->keys, i.e. there's no need for this temporary variable. Johan