| Message ID | 1408699567-6940-2-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 08/22/2014 03:26 AM, Peter Lieven wrote: > regular bitmap_new simply aborts if the memory allocation fails. > bitmap_try_new returns NULL on failure and allows for proper > error handling. > > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > include/qemu/bitmap.h | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h > index 1babd5d..51b430f 100644 > --- a/include/qemu/bitmap.h > +++ b/include/qemu/bitmap.h > @@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits) > return g_malloc0(len); > } > > +static inline unsigned long *bitmap_try_new(long nbits) > +{ > + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > + return g_try_malloc0(len); > +} > + What you have works, but I personally would have reimplemented bitmap_new as the first caller of bitmap_try_new in this patch, where bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that it has the same behavior. By routing the one function to use the other, we are future-proofing: if initialization of a bitmap ever needs modification, we only update bitmap_try_new, rather than copying the updates to both functions.
Am 25.08.2014 um 17:09 schrieb Eric Blake: > On 08/22/2014 03:26 AM, Peter Lieven wrote: >> regular bitmap_new simply aborts if the memory allocation fails. >> bitmap_try_new returns NULL on failure and allows for proper >> error handling. >> >> Signed-off-by: Peter Lieven <pl@kamp.de> >> --- >> include/qemu/bitmap.h | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h >> index 1babd5d..51b430f 100644 >> --- a/include/qemu/bitmap.h >> +++ b/include/qemu/bitmap.h >> @@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits) >> return g_malloc0(len); >> } >> >> +static inline unsigned long *bitmap_try_new(long nbits) >> +{ >> + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); >> + return g_try_malloc0(len); >> +} >> + > What you have works, but I personally would have reimplemented > bitmap_new as the first caller of bitmap_try_new in this patch, where > bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that > it has the same behavior. By routing the one function to use the other, > we are future-proofing: if initialization of a bitmap ever needs > modification, we only update bitmap_try_new, rather than copying the > updates to both functions. > Good point. What would be the right exit if we receive a NULL from bitmap_try_new? abort() ? Peter
On 09/05/2014 02:00 PM, Peter Lieven wrote: >>> + >> What you have works, but I personally would have reimplemented >> bitmap_new as the first caller of bitmap_try_new in this patch, where >> bitmap_new handles a NULL bitmap_try_new return by abort()ing, so that >> it has the same behavior. By routing the one function to use the other, >> we are future-proofing: if initialization of a bitmap ever needs >> modification, we only update bitmap_try_new, rather than copying the >> updates to both functions. >> > > Good point. What would be the right exit if we receive a NULL from > bitmap_try_new? abort() ? I think that g_malloc() calls abort(), so yes, that sounds right.
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h index 1babd5d..51b430f 100644 --- a/include/qemu/bitmap.h +++ b/include/qemu/bitmap.h @@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits) return g_malloc0(len); } +static inline unsigned long *bitmap_try_new(long nbits) +{ + long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); + return g_try_malloc0(len); +} + static inline void bitmap_zero(unsigned long *dst, long nbits) { if (small_nbits(nbits)) {
regular bitmap_new simply aborts if the memory allocation fails. bitmap_try_new returns NULL on failure and allows for proper error handling. Signed-off-by: Peter Lieven <pl@kamp.de> --- include/qemu/bitmap.h | 6 ++++++ 1 file changed, 6 insertions(+)