A patch to stamp talloc_move/talloc_steal info ...

[Simo]

On Thu, 2016-03-03 at 10:19 -0800, Richard Sharpe wrote:
> Hi folks,
> 
> I talked about this a while ago.
> 
> Here is a patch.
> 
> I will try to send a properly formatted git patch soon.
> 
> diff -ur talloc-2.1.1/talloc.c talloc-2.1.1-mod/talloc.c
> --- talloc-2.1.1/talloc.c       2016-02-13 13:34:47.266100765 -0800
> +++ talloc-2.1.1-mod/talloc.c   2016-02-11 18:00:18.858928133 -0800
> @@ -273,6 +273,8 @@
>          * from.
>          */
>         struct talloc_pool_hdr *pool;
> +
> +        const char *stolen_loc;
>  };
> 
>  /* 16 byte alignment seems to keep everyone happy */
> @@ -850,7 +852,7 @@
>         return handle->ptr;
>  }
> 
> -static void *_talloc_steal_internal(const void *new_ctx, const void
> *ptr);
> +static void *_talloc_steal_internal(const void *new_ctx, const void
> *ptr, const char *location);
> 
>  static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
>                                         const char *location)
> @@ -1067,7 +1069,7 @@
>     ptr on success, or NULL if it could not be transferred.
>     passing NULL as ptr will always return NULL with no side effects.
>  */
> -static void *_talloc_steal_internal(const void *new_ctx, const void
> *ptr)
> +static void *_talloc_steal_internal(const void *new_ctx, const void
> *ptr, const char *location)
>  {
>         struct talloc_chunk *tc, *new_tc;
>         size_t ctx_size = 0;
> @@ -1082,6 +1084,11 @@
> 
>         tc = talloc_chunk_from_ptr(ptr);
> 
> +       /* Stamp the name regardless */
> +       if (location) {
> +               tc->stolen_loc = location;
> +        }
> +
>         if (tc->limit != NULL) {
> 
>                 ctx_size = _talloc_total_limit_size(ptr, NULL, NULL);
> @@ -1179,7 +1186,7 @@
>         }
>  #endif
> 
> -       return _talloc_steal_internal(new_ctx, ptr);
> +       return _talloc_steal_internal(new_ctx, ptr, location);
>  }
> 
>  /*
> @@ -1199,13 +1206,13 @@
>         }
> 
>         if (old_parent == talloc_parent(ptr)) {
> -               return _talloc_steal_internal(new_parent, ptr);
> +               return _talloc_steal_internal(new_parent, ptr, NULL);
>         }
> 
>         tc = talloc_chunk_from_ptr(ptr);
>         for (h=tc->refs;h;h=h->next) {
>                 if (talloc_parent(h) == old_parent) {
> -                       if (_talloc_steal_internal(new_parent, h) !=
> h) {
> +                       if (_talloc_steal_internal(new_parent, h,
> NULL) != h) {
>                                 return NULL;
>                         }
>                         return discard_const_p(void, ptr);
> @@ -1292,7 +1299,7 @@
>                 return -1;
>         }
> 
> -       _talloc_steal_internal(new_parent, ptr);
> +       _talloc_steal_internal(new_parent, ptr, NULL);
> 
>         return 0;
>  }
> @@ -1468,7 +1475,7 @@
>                                 struct talloc_chunk *p =
> talloc_parent_chunk(ptr);
>                                 if (p) new_parent =
> TC_PTR_FROM_CHUNK(p);
>                         }
> -                       _talloc_steal_internal(new_parent, child);
> +                       _talloc_steal_internal(new_parent, child,
> location);
>                 }
>         }
>  }
> @@ -1855,10 +1862,10 @@
>    a wrapper around talloc_steal() for situations where you are
> moving a pointer
>    between two structures, and want the old pointer to be set to NULL
>  */
> -_PUBLIC_ void *_talloc_move(const void *new_ctx, const void *_pptr)
> +_PUBLIC_ void *_talloc_move(const void *new_ctx, const void *_pptr,
> const char *location)
>  {
>         const void **pptr = discard_const_p(const void *,_pptr);
> -       void *ret = talloc_steal(new_ctx, discard_const_p(void,
> *pptr));
> +       void *ret = _talloc_steal_loc(new_ctx, discard_const_p(void,
> *pptr), location);
>         (*pptr) = NULL;
>         return ret;
>  }
> @@ -2059,9 +2066,9 @@
>                 return;
>         }
> 
> -       fprintf(f, "%*s%-30s contains %6lu bytes in %3lu blocks (ref
> %d) %p\n",
> +       fprintf(f, "%*s%-30s at %p contains %6lu bytes in %3lu blocks
> (ref %d) %p\n",
>                 depth*4, "",
> -               name,
> +               name, ptr,
>                 (unsigned long)talloc_total_size(ptr),
>                 (unsigned long)talloc_total_blocks(ptr),
>                 (int)talloc_reference_count(ptr), ptr);
> diff -ur talloc-2.1.1/talloc.h talloc-2.1.1-mod/talloc.h
> --- talloc-2.1.1/talloc.h       2014-05-20 05:32:13.000000000 -0700
> +++ talloc-2.1.1-mod/talloc.h   2016-02-11 17:49:58.627789375 -0800
> @@ -412,8 +412,8 @@
>   */
>  void *talloc_move(const void *new_ctx, void **pptr);
>  #else
> -#define talloc_move(ctx, pptr)
> (_TALLOC_TYPEOF(*(pptr)))_talloc_move((ctx),(void *)(pptr))
> -void *_talloc_move(const void *new_ctx, const void *pptr);
> +#define talloc_move(ctx, pptr)
> (_TALLOC_TYPEOF(*(pptr)))_talloc_move((ctx),(void *)(pptr),
> __location__)
> +void *_talloc_move(const void *new_ctx, const void *pptr, const char
> *location);
>  #endif
> 
>  /**
> 
> 

NACK, this is an ABI break, and the gain doesn't really look it is
worth breaking everything out there.

If you really want to do this you have to keep _talloc_move as is, add
a new function and change the header to use the new one, but still
expose the old one in the library.

Simo.