Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy

[Szymon Janc <szymon.janc@tieto.com>]

Hi Bharat,

On Friday 29 of May 2015 20:26:40 Bharat Bhusan Panda wrote:
> Hi Szymon,
> 
> > -----Original Message-----
> > From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> > owner@vger.kernel.org] On Behalf Of Szymon Janc
> > Sent: Friday, May 29, 2015 8:10 PM
> > To: Bharat Panda
> > Cc: linux-bluetooth@vger.kernel.org; cpgs@samsung.com
> > Subject: Re: [PATCH ] unit: Use g_memdup replacing g_malloc0+memcpy
> > 
> > Hi Bharat,
> > 
> > On Friday 29 of May 2015 17:18:10 Bharat Panda wrote:
> > > Replace use of g_malloc0+memcpy with g_memdup.
> > > ---
> > > 
> > >  unit/test-gattrib.c | 3 +--
> > >  unit/test-sdp.c     | 4 +---
> > >  2 files changed, 2 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/unit/test-gattrib.c b/unit/test-gattrib.c index
> > > 416e596..d9c52d7 100644
> > > --- a/unit/test-gattrib.c
> > > +++ b/unit/test-gattrib.c
> > > @@ -224,8 +224,7 @@ static void result_canary(guint8 status, const
> > > guint8 *pdu, guint16 len, struct result_data *result = data;
> > > 
> > >  	result->status = status;
> > > 
> > > -	result->pdu = g_malloc0(len);
> > > -	memcpy(result->pdu, pdu, len);
> > > +	result->pdu = g_memdup(pdu, len);
> > > 
> > >  	result->len = len;
> > >  	
> > >  	if (g_test_verbose())
> > > 
> > > diff --git a/unit/test-sdp.c b/unit/test-sdp.c index b4ef4d1..58d90f8
> > > 100644
> > > --- a/unit/test-sdp.c
> > > +++ b/unit/test-sdp.c
> > > @@ -189,9 +189,7 @@ static gboolean send_pdu(gpointer user_data)
> > > 
> > >  	pdu_len = req_pdu->raw_size + context->cont_size;
> > > 
> > > -	buf = g_malloc0(pdu_len);
> > > -
> > > -	memcpy(buf, req_pdu->raw_data, req_pdu->raw_size);
> > > +	buf = g_memdup(req_pdu->raw_data, req_pdu->raw_size);
> > 
> > This is not correct. If context->cont_size > 0 then you write after
> 
> allocated
> 
> > memory (see line below).
> 
> Below line is to copy the context->cont_data after req_pdu->raw_size length
> of req_pdu->raw_data.
> But in above lines, it was allocating pdu_len size then doing memcpy of
> req_pdu->raw_data, which was replaced by the g_memdup.

Yes. So instead of allocating pdu_len bytes you allocate only
req_pdu->raw_size bytes. This is not enough if context->cont_size is greater 
than zero. You have second memcpy just line below.

> > >  	if (context->cont_size > 0)
> > >  	
> > >  		memcpy(buf + req_pdu->raw_size, context->cont_data,

This will write after allocated memory.

> 
> --
> Bharat
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth"
> in the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
BR
Szymon Janc