Quoting Wolfgang Bumiller ([email protected]): > > > On January 7, 2016 at 7:42 PM Serge Hallyn <[email protected]> wrote: > > > > > > Quoting Wolfgang Bumiller ([email protected]): > > > The initial check should use real lengths as with modulo a > > > new required length of eg. 52 would be considered smaller > > > than an old length of 48 (2 < 48). > > > > > > To get the 'batches' count 'newlen' must be divided and not > > > taken modulo BATCH_SIZE. Otherwise '101', which would need a > > > 3rd batch to reach 150, would end up with two (2*50 = 100 > > > bytes) and thereby be truncated instead. > > > > > > Signed-off-by: Wolfgang Bumiller <[email protected]> > > > --- > > > cgfs.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/cgfs.c b/cgfs.c > > > index 0659e9e..681a478 100644 > > > --- a/cgfs.c > > > +++ b/cgfs.c > > > @@ -75,9 +75,9 @@ static inline void drop_trailing_newlines(char *s) > > > static void dorealloc(char **mem, size_t oldlen, size_t newlen) > > > { > > > int batches; > > > - if (newlen % BATCH_SIZE <= oldlen % BATCH_SIZE) > > > + if (newlen <= oldlen) > > > > This will result in extra reallocs (though innocuous). > > Is there any reason not to just do > > > > if (newlen / BATCH_SIZE <= oldlen / BATCH_SIZE) > > Ah yes I missed that the old length is a string length not the > capacity, so yes, the division makes sense. Just not the modulus. >
Ok, thanks - per your patch I think the following makes a nice cleanup: From a72193ff0dba319c3039372bc038708a75d0582f Mon Sep 17 00:00:00 2001 From: Serge Hallyn <[email protected]> Date: Thu, 7 Jan 2016 11:17:17 -0800 Subject: [PATCH 1/1] dorealloc: avoid extra reallocs The original check was very wrong, using % instead of /. However the length we track is the actual used length, not the allocated length, which is always (len / BATCH_SIZE) + 1. We don't want to realloc when newlen is between oldlen and (oldlen / BATCH_SIZE) + 1) Signed-off-by: Serge Hallyn <[email protected]> --- cgfs.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/cgfs.c b/cgfs.c index ec0f630..f37bc2b 100644 --- a/cgfs.c +++ b/cgfs.c @@ -74,18 +74,20 @@ static inline void drop_trailing_newlines(char *s) #define BATCH_SIZE 50 static void dorealloc(char **mem, size_t oldlen, size_t newlen) { - int batches; - if (newlen <= oldlen) + int newbatches = (newlen / BATCH_SIZE) + 1; + int oldbatches = (olden / BATCH_SIZE) + 1; + + if (newbatches <= oldbatches) return; - batches = (newlen / BATCH_SIZE) + 1; + if (!*mem) { do { - *mem = malloc(batches * BATCH_SIZE); + *mem = malloc(newbatches * BATCH_SIZE); } while (!*mem); } else { char *tmp; do { - tmp = realloc(*mem, batches * BATCH_SIZE); + tmp = realloc(*mem, newbatches * BATCH_SIZE); } while (!tmp); *mem = tmp; } -- 2.5.0 _______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
