| Message ID | 1496950247-8755-4-git-send-email-sgoel@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
CC the REST maintainers On 08/06/2017 20:30, Sameer Goel wrote: > Introduce a memory realloc function. > > Signed-off-by: Sameer Goel <sgoel@codeaurora.org> > --- > xen/common/xmalloc_tlsf.c | 13 +++++++++++++ > xen/include/xen/xmalloc.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c > index b256dc5..52385a8 100644 > --- a/xen/common/xmalloc_tlsf.c > +++ b/xen/common/xmalloc_tlsf.c > @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) > return p ? memset(p, 0, size) : p; > } > > +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) > +{ > + void *new_p = _xmalloc(new_size, align); > + > + if(new_p && p) Coding style: if ( ... ) > + { > + memcpy(new_p, p, new_size); > + xfree(p); > + } > + > + return new_p; > +} > + > void xfree(void *p) > { > struct bhdr *b; > diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h > index 24a99ac..41a9b2f 100644 > --- a/xen/include/xen/xmalloc.h > +++ b/xen/include/xen/xmalloc.h > @@ -29,6 +29,7 @@ extern void xfree(void *); > /* Underlying functions */ > extern void *_xmalloc(unsigned long size, unsigned long align); > extern void *_xzalloc(unsigned long size, unsigned long align); > +extern void *_xrealloc(void *p, unsigned long new_size, unsigned long align); > > static inline void *_xmalloc_array( > unsigned long size, unsigned long align, unsigned long num) >
On Thu, 8 Jun 2017, Sameer Goel wrote: > Introduce a memory realloc function. > > Signed-off-by: Sameer Goel <sgoel@codeaurora.org> > --- > xen/common/xmalloc_tlsf.c | 13 +++++++++++++ > xen/include/xen/xmalloc.h | 1 + > 2 files changed, 14 insertions(+) > > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c > index b256dc5..52385a8 100644 > --- a/xen/common/xmalloc_tlsf.c > +++ b/xen/common/xmalloc_tlsf.c > @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) > return p ? memset(p, 0, size) : p; > } > > +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) > +{ > + void *new_p = _xmalloc(new_size, align); It might be best to handle the case where new_size is 0 explicitly, and only free p. > + if(new_p && p) > + { > + memcpy(new_p, p, new_size); > + xfree(p); > + } > + > + return new_p; > +} > + > void xfree(void *p) > { > struct bhdr *b; > diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h > index 24a99ac..41a9b2f 100644 > --- a/xen/include/xen/xmalloc.h > +++ b/xen/include/xen/xmalloc.h > @@ -29,6 +29,7 @@ extern void xfree(void *); > /* Underlying functions */ > extern void *_xmalloc(unsigned long size, unsigned long align); > extern void *_xzalloc(unsigned long size, unsigned long align); > +extern void *_xrealloc(void *p, unsigned long new_size, unsigned long align); > > static inline void *_xmalloc_array( > unsigned long size, unsigned long align, unsigned long num) > -- > Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. >
On Thu, Jun 08, 2017 at 08:49:01PM +0100, Julien Grall wrote: > CC the REST maintainers > > On 08/06/2017 20:30, Sameer Goel wrote: > > Introduce a memory realloc function. > > > > Signed-off-by: Sameer Goel <sgoel@codeaurora.org> > > --- > > xen/common/xmalloc_tlsf.c | 13 +++++++++++++ > > xen/include/xen/xmalloc.h | 1 + > > 2 files changed, 14 insertions(+) > > > > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c > > index b256dc5..52385a8 100644 > > --- a/xen/common/xmalloc_tlsf.c > > +++ b/xen/common/xmalloc_tlsf.c > > @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) > > return p ? memset(p, 0, size) : p; > > } > > > > +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) > > +{ > > + void *new_p = _xmalloc(new_size, align); > > + > > + if(new_p && p) > > Coding style: if ( ... ) > > > + { > > + memcpy(new_p, p, new_size); This is wrong. How can you know if the area pointed to by p is at least new_size bytes long?
On 6/9/2017 3:44 AM, Wei Liu wrote: > On Thu, Jun 08, 2017 at 08:49:01PM +0100, Julien Grall wrote: >> CC the REST maintainers >> >> On 08/06/2017 20:30, Sameer Goel wrote: >>> Introduce a memory realloc function. >>> >>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org> >>> --- >>> xen/common/xmalloc_tlsf.c | 13 +++++++++++++ >>> xen/include/xen/xmalloc.h | 1 + >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c >>> index b256dc5..52385a8 100644 >>> --- a/xen/common/xmalloc_tlsf.c >>> +++ b/xen/common/xmalloc_tlsf.c >>> @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) >>> return p ? memset(p, 0, size) : p; >>> } >>> >>> +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) >>> +{ >>> + void *new_p = _xmalloc(new_size, align); >>> + >>> + if(new_p && p) >> >> Coding style: if ( ... ) >> >>> + { >>> + memcpy(new_p, p, new_size); > > This is wrong. How can you know if the area pointed to by p is at least > new_size bytes long? > Agreed, I revisited the code and will remove _xrealloc and use xfree and _xmalloc instead. Thanks, Sameer
Hi, Bringing back this discussion. On 28/08/17 22:39, Goel, Sameer wrote: > > > On 6/9/2017 3:44 AM, Wei Liu wrote: >> On Thu, Jun 08, 2017 at 08:49:01PM +0100, Julien Grall wrote: >>> CC the REST maintainers >>> >>> On 08/06/2017 20:30, Sameer Goel wrote: >>>> Introduce a memory realloc function. >>>> >>>> Signed-off-by: Sameer Goel <sgoel@codeaurora.org> >>>> --- >>>> xen/common/xmalloc_tlsf.c | 13 +++++++++++++ >>>> xen/include/xen/xmalloc.h | 1 + >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c >>>> index b256dc5..52385a8 100644 >>>> --- a/xen/common/xmalloc_tlsf.c >>>> +++ b/xen/common/xmalloc_tlsf.c >>>> @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) >>>> return p ? memset(p, 0, size) : p; >>>> } >>>> >>>> +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) >>>> +{ >>>> + void *new_p = _xmalloc(new_size, align); >>>> + >>>> + if(new_p && p) >>> >>> Coding style: if ( ... ) >>> >>>> + { >>>> + memcpy(new_p, p, new_size); >> >> This is wrong. How can you know if the area pointed to by p is at least >> new_size bytes long? >> > Agreed, I revisited the code and will remove _xrealloc and use xfree and _xmalloc instead. I am not sure why you choose to drop it completely. xfree is able to know the size of the buffer to free. So you could find out the size and copy the right amount of data. Note that having _xrealloc would be my preference over an open-coded version in the code. Cheers,
On Thu, Oct 12, 2017 at 02:33:04PM +0100, Julien Grall wrote: > Hi, > > Bringing back this discussion. > > On 28/08/17 22:39, Goel, Sameer wrote: > > > > > > On 6/9/2017 3:44 AM, Wei Liu wrote: > > > On Thu, Jun 08, 2017 at 08:49:01PM +0100, Julien Grall wrote: > > > > CC the REST maintainers > > > > > > > > On 08/06/2017 20:30, Sameer Goel wrote: > > > > > Introduce a memory realloc function. > > > > > > > > > > Signed-off-by: Sameer Goel <sgoel@codeaurora.org> > > > > > --- > > > > > xen/common/xmalloc_tlsf.c | 13 +++++++++++++ > > > > > xen/include/xen/xmalloc.h | 1 + > > > > > 2 files changed, 14 insertions(+) > > > > > > > > > > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c > > > > > index b256dc5..52385a8 100644 > > > > > --- a/xen/common/xmalloc_tlsf.c > > > > > +++ b/xen/common/xmalloc_tlsf.c > > > > > @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) > > > > > return p ? memset(p, 0, size) : p; > > > > > } > > > > > > > > > > +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) > > > > > +{ > > > > > + void *new_p = _xmalloc(new_size, align); > > > > > + > > > > > + if(new_p && p) > > > > > > > > Coding style: if ( ... ) > > > > > > > > > + { > > > > > + memcpy(new_p, p, new_size); > > > > > > This is wrong. How can you know if the area pointed to by p is at least > > > new_size bytes long? > > > > > Agreed, I revisited the code and will remove _xrealloc and use xfree and _xmalloc instead. > > I am not sure why you choose to drop it completely. xfree is able to know > the size of the buffer to free. > > So you could find out the size and copy the right amount of data. > > Note that having _xrealloc would be my preference over an open-coded version > in the code. I would vouch for a properly implemented _xrealloc. :-)
diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c index b256dc5..52385a8 100644 --- a/xen/common/xmalloc_tlsf.c +++ b/xen/common/xmalloc_tlsf.c @@ -612,6 +612,19 @@ void *_xzalloc(unsigned long size, unsigned long align) return p ? memset(p, 0, size) : p; } +void *_xrealloc(void *p, unsigned long new_size, unsigned long align) +{ + void *new_p = _xmalloc(new_size, align); + + if(new_p && p) + { + memcpy(new_p, p, new_size); + xfree(p); + } + + return new_p; +} + void xfree(void *p) { struct bhdr *b; diff --git a/xen/include/xen/xmalloc.h b/xen/include/xen/xmalloc.h index 24a99ac..41a9b2f 100644 --- a/xen/include/xen/xmalloc.h +++ b/xen/include/xen/xmalloc.h @@ -29,6 +29,7 @@ extern void xfree(void *); /* Underlying functions */ extern void *_xmalloc(unsigned long size, unsigned long align); extern void *_xzalloc(unsigned long size, unsigned long align); +extern void *_xrealloc(void *p, unsigned long new_size, unsigned long align); static inline void *_xmalloc_array( unsigned long size, unsigned long align, unsigned long num)
Introduce a memory realloc function. Signed-off-by: Sameer Goel <sgoel@codeaurora.org> --- xen/common/xmalloc_tlsf.c | 13 +++++++++++++ xen/include/xen/xmalloc.h | 1 + 2 files changed, 14 insertions(+)