This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Fwd: [PATCH] tdestroy() should allow passing 'NULL' for 'freefct' callback
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Ivo Raisr <ivosh at ivosh dot net>
- Cc: libc-alpha at sourceware dot org, OndÅej BÃlka <neleai at seznam dot cz>
- Date: Tue, 25 Jun 2013 11:42:46 -0400
- Subject: Re: Fwd: [PATCH] tdestroy() should allow passing 'NULL' for 'freefct' callback
- References: <CANXv6=sw64PRodqTgArqUHQLTcQd8eEE_==6QHVHgLn52jh=1Q at mail dot gmail dot com> <20130611081615 dot GA6224 at domone dot kolej dot mff dot cuni dot cz> <CANXv6=vaJwfOKGgNWU55j-xdB5zqhp5_-3tdjS287+UR1r-nDA at mail dot gmail dot com> <CANXv6=sKC3wsF5ZN9So9Pq=BAfcwvveNZHaSvWEZL5KceH+goQ at mail dot gmail dot com>
Ivor,
This is just under what I would call legally significant changes.
Please consider applying for futures copyright assignment with the FSF
for glibc so that we can more easily and quickly incoporate your fixes.
Cheers,
Carlos.
On 06/25/2013 12:00 AM, Ivo Raisr wrote:
> I haven't got any response yet...
> I.
>
>
> ---------- Forwarded message ----------
> From: Ivo Raisr <ivosh@ivosh.net>
> Date: 2013/6/12
> Subject: Re: [PATCH] tdestroy() should allow passing 'NULL' for
> 'freefct' callback
> To: libc-alpha@sourceware.org
>
> ...
> New patch (v2) is attached.
> ...
>
> Ivosh
>
>
> glibc-tdestroy-freefct-NULL-v2.patch
>
>
> 2013-05-24 Ivo Raisr ivosh@ivosh.net
>
> [BZ #15009]
> * misc/tsearch.c (__tdestroy): Argument freefct can be 'NULL'.
s/Argument freefct can be 'NULL'./freefct can be NULL/g.
> * elf/sprof.c: Dummy function 'freenoop' removed.
s/Dummy function 'freenoop' removed./Removed freenoop./g
> * manual/search.texi: Documentation update.
>
> diff -urN glibc-orig/elf/sprof.c glibc/elf/sprof.c
> --- glibc-orig/elf/sprof.c 2013-05-23 14:34:35.639576492 -0700
> +++ glibc/elf/sprof.c 2013-05-23 14:35:15.955575664 -0700
> @@ -1300,13 +1300,6 @@
> }
>
>
> -/* ARGUSED */
> -static void
> -freenoop (void *p)
> -{
> -}
> -
> -
OK.
> static void
> generate_flat_profile (struct profdata *profdata)
> {
> @@ -1328,7 +1321,7 @@
>
> twalk (data, printflat);
>
> - tdestroy (data, freenoop);
> + tdestroy (data, NULL);
OK.
> }
>
>
> diff -urN glibc-orig/manual/search.texi glibc/manual/search.texi
> --- glibc-orig/manual/search.texi 2013-05-23 14:34:35.439576496 -0700
> +++ glibc/manual/search.texi 2013-05-23 14:39:22.931570596 -0700
> @@ -498,8 +498,7 @@
>
> For the data in each tree node the function @var{freefct} is called.
> The pointer to the data is passed as the argument to the function. If
> -no such work is necessary @var{freefct} must point to a function doing
> -nothing. It is called in any case.
> +no such work is necessary @var{freefct} can be @code{NULL}.
>
> This function is a GNU extension and not covered by the @w{System V} or
> X/Open specifications.
> diff -urN glibc-orig/misc/tsearch.c glibc/misc/tsearch.c
> --- glibc-orig/misc/tsearch.c 2013-05-23 14:34:35.711576491 -0700
> +++ glibc/misc/tsearch.c 2013-06-12 04:25:30.764454483 -0700
> @@ -644,6 +644,13 @@
> free (root);
> }
>
> +/* ARGUSED */
> +static void
> +internal_function
> +free_noop (void *dummy)
> +{
> +}
> +
OK.
> void
> __tdestroy (void *vroot, __free_fn_t freefct)
> {
> @@ -652,6 +659,10 @@
> CHECK_TREE (root);
>
> if (root != NULL)
> - tdestroy_recurse (root, freefct);
> + {
> + if (freefct == NULL)
This needs a __glibc_unlikely use here to help branch prediction.
It's unlikely you'll have a null freefct.
> + freefct = free_noop;
> + tdestroy_recurse (root, freefct);
> + }
> }
> weak_alias (__tdestroy, tdestroy)
Overall I like the patch, it makes the interface a little easier
to use. There is a slight cost ot having the empty function, but
that's minor.
Please post a v3.
Cheers,
Carlos.