[Mesa-dev] [PATCH 7/7] util: Fix ralloc to use malloc instead of calloc

[Juha-Pekka Heikkila]

On 14.06.2016 18:03, Jason Ekstrand wrote:
>
>
> On Tue, Jun 14, 2016 at 7:59 AM, Juha-Pekka Heikkila
> <juhapekka.heikkila at gmail.com <mailto:juhapekka.heikkila at gmail.com>> wrote:
>
>     ralloc originally had had idea of using malloc but somehow
>     had slipped in calloc. Without these changes rzalloc did double
>     job of zeroing the memory, first calloc and then memset.
>     Now change ralloc to use malloc, leave rzalloc to use calloc and
>     make needed changes in ralloc functions to get things working.
>
>     Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com
>     <mailto:juhapekka.heikkila at gmail.com>>
>     ---
>       src/util/ralloc.c | 49
>     +++++++++++++++++++++++++++++++++----------------
>       src/util/ralloc.h |  2 +-
>       2 files changed, 34 insertions(+), 17 deletions(-)
>
>     diff --git a/src/util/ralloc.c b/src/util/ralloc.c
>     index 9526011..527385d 100644
>     --- a/src/util/ralloc.c
>     +++ b/src/util/ralloc.c
>     @@ -104,25 +104,12 @@ add_child(ralloc_header *parent, ralloc_header
>     *info)
>       void *
>       ralloc_context(const void *ctx)
>       {
>     -   return ralloc_size(ctx, 0);
>     +   return rzalloc_size(ctx, 0);
>       }
>
>     -void *
>     -ralloc_size(const void *ctx, size_t size)
>     -{
>     -   /* ralloc_size was originally implemented using calloc, which
>     meant some
>     -    * code accidentally relied on its zero filling behavior.
>     -    *
>     -    * TODO: Make ralloc_size not zero fill memory, and cleanup any
>     code that
>     -    * should instead be using rzalloc.
>     -    */
>     -   return rzalloc_size(ctx, size);
>     -}
>     -
>     -void *
>     -rzalloc_size(const void *ctx, size_t size)
>     +static void*
>     +ralloc_header_helper(const void *ctx, const void *block)
>       {
>     -   void *block = calloc(1, size + sizeof(ralloc_header));
>          ralloc_header *info;
>          ralloc_header *parent;
>
>     @@ -131,6 +118,12 @@ rzalloc_size(const void *ctx, size_t size)
>          info = (ralloc_header *) block;
>          parent = ctx != NULL ? get_header(ctx) : NULL;
>
>     +   info->child = NULL;
>     +   info->parent = NULL;
>     +   info->prev = NULL;
>     +   info->next = NULL;
>     +   info->destructor = NULL;
>     +
>          add_child(parent, info);
>
>       #ifdef DEBUG
>     @@ -140,6 +133,30 @@ rzalloc_size(const void *ctx, size_t size)
>          return PTR_FROM_HEADER(info);
>       }
>
>     +void *
>     +ralloc_size(const void *ctx, size_t size)
>     +{
>     +   void *block;
>     +
>     +   if (size + sizeof(ralloc_header) < size )
>     +      return NULL;
>     +
>     +   block = malloc(size + sizeof(ralloc_header));
>
>
> One more thing that I think I'd like to see you try is to add
>
> memset(block, 139, size + sizeof(ralloc_header));
>
> right here and see if everything still works.  Frequently malloc will
> return zerod memory and we'd like to be 100% sure that isn't a problem.
> For that matter, I think it'd be good to just put that inside of a
> "#ifndef NDEBUG" and leave it so that it gets tested in debug builds.

Both these I think are great ideas, thanks Jason. For IVB I'm probably 
good with Valgrind but for rest of the platforms I cannot go run 
Valgrind in Mark's test system.

As for those other platforms (freedreno and others) that I cannot go 
test I hope to have time to look at over weekend and be back with new 
set again next week.

>
>     +   return ralloc_header_helper(ctx, block);
>     +}
>     +
>     +void *
>     +rzalloc_size(const void *ctx, size_t size)
>     +{
>     +   void *block;
>     +
>     +   if (size + sizeof(ralloc_header) < size )
>     +      return NULL;
>     +
>     +   block = calloc(1, size + sizeof(ralloc_header));
>     +   return ralloc_header_helper(ctx, block);
>     +}
>     +
>       /* helper function - assumes ptr != NULL */
>       static void *
>       resize(void *ptr, size_t size)
>     diff --git a/src/util/ralloc.h b/src/util/ralloc.h
>     index 7587e11..462db7d 100644
>     --- a/src/util/ralloc.h
>     +++ b/src/util/ralloc.h
>     @@ -430,7 +430,7 @@ private:
>                               \
>       public:
>            \
>          static void* operator new(size_t size, void *mem_ctx)
>             \
>          {
>             \
>     -      void *p = ralloc_size(mem_ctx, size);
>            \
>     +      void *p = rzalloc_size(mem_ctx, size);
>           \
>             assert(p != NULL);
>             \
>             if (!HAS_TRIVIAL_DESTRUCTOR(TYPE))
>             \
>                ralloc_set_destructor(p, _ralloc_destructor);
>             \
>     --
>     1.9.1
>
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>