You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Igor Galić <i....@brainsware.org> on 2014/06/20 20:16:11 UTC
Re: git commit: TS-2689: Remove extraneous TSfree that caused
segfault
----- Original Message -----
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 2c9a15cec -> c524d54cb
>
>
> TS-2689: Remove extraneous TSfree that caused segfault
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c524d54c
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c524d54c
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c524d54c
>
> Branch: refs/heads/master
> Commit: c524d54cb966180810a5de5c59271bd68829f149
> Parents: 2c9a15c
> Author: Justin Laue <ju...@fp-x.com>
> Authored: Thu Jun 19 16:23:59 2014 -0600
> Committer: Phil Sorber <so...@apache.org>
> Committed: Thu Jun 19 16:43:14 2014 -0600
>
> ----------------------------------------------------------------------
> CHANGES | 2 ++
> plugins/cacheurl/cacheurl.cc | 1 -
> 2 files changed, 2 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c524d54c/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 2965aac..764cb9a 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,8 @@
> -*- coding: utf-8
> -*-
> Changes with Apache Traffic Server 5.1.0
>
> + *) [TS-2689] Remove extraneous TSfree that caused segfault.
> +
> *) [TS-2892] Keep alive post out enabled by default
>
> *) [TS-2889] Crash in FetchSM related to spdy FetchSM changes in 5.0.x
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c524d54c/plugins/cacheurl/cacheurl.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/cacheurl/cacheurl.cc b/plugins/cacheurl/cacheurl.cc
> index c32d52d..69feb06 100644
> --- a/plugins/cacheurl/cacheurl.cc
> +++ b/plugins/cacheurl/cacheurl.cc
> @@ -403,7 +403,6 @@ TSRemapDeleteInstance(void *ih)
>
> TSDebug(PLUGIN_NAME, "Deleting remap instance");
>
> - TSfree(prl);
> delete prl;
> }
>
This is unrelated, but what's wrong with just NULLing the
pointers in our ats_free() functions?
diff --git lib/ts/ink_memory.cc lib/ts/ink_memory.cc
index 91aa403..2cca892 100644
--- lib/ts/ink_memory.cc
+++ lib/ts/ink_memory.cc
@@ -119,23 +119,22 @@ ats_memalign(size_t alignment, size_t size)
void
ats_free(void *ptr)
{
- if (likely(ptr != NULL))
- free(ptr);
+ ats_free_null(ptr);
} /* End ats_free */
void*
ats_free_null(void *ptr)
{
- if (likely(ptr != NULL))
- free(ptr);
+ free(ptr);
+ ptr = NULL;
return NULL;
} /* End ats_free_null */
void
ats_memalign_free(void *ptr)
{
- if (likely(ptr))
- free(ptr);
+ free(ptr);
+ ptr = NULL;
}
// This effectively makes mallopt() a no-op (currently) when tcmalloc
free(3) should be able to handle that NULL pointer all right by itself,
and we are making sure to NULL it, such that potential reuse may be noticed.
-- i
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.galic@brainsware.org
URL: http://brainsware.org/
GPG: 8716 7A9F 989B ABD5 100F 4008 F266 55D6 2998 1641
Re: git commit: TS-2689: Remove extraneous TSfree that caused segfault
Posted by Leif Hedstrom <zw...@apache.org>.
> On Jun 20, 2014, at 12:16 PM, Igor Galić <i....@brainsware.org> wrote:
>
>
>
> ----- Original Message -----
>> Repository: trafficserver
>> Updated Branches:
>> refs/heads/master 2c9a15cec -> c524d54cb
>>
>>
>> TS-2689: Remove extraneous TSfree that caused segfault
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/c524d54c
>> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/c524d54c
>> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/c524d54c
>>
>> Branch: refs/heads/master
>> Commit: c524d54cb966180810a5de5c59271bd68829f149
>> Parents: 2c9a15c
>> Author: Justin Laue <ju...@fp-x.com>
>> Authored: Thu Jun 19 16:23:59 2014 -0600
>> Committer: Phil Sorber <so...@apache.org>
>> Committed: Thu Jun 19 16:43:14 2014 -0600
>>
>> ----------------------------------------------------------------------
>> CHANGES | 2 ++
>> plugins/cacheurl/cacheurl.cc | 1 -
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c524d54c/CHANGES
>> ----------------------------------------------------------------------
>> diff --git a/CHANGES b/CHANGES
>> index 2965aac..764cb9a 100644
>> --- a/CHANGES
>> +++ b/CHANGES
>> @@ -1,6 +1,8 @@
>> -*- coding: utf-8
>> -*-
>> Changes with Apache Traffic Server 5.1.0
>>
>> + *) [TS-2689] Remove extraneous TSfree that caused segfault.
>> +
>> *) [TS-2892] Keep alive post out enabled by default
>>
>> *) [TS-2889] Crash in FetchSM related to spdy FetchSM changes in 5.0.x
>>
>> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/c524d54c/plugins/cacheurl/cacheurl.cc
>> ----------------------------------------------------------------------
>> diff --git a/plugins/cacheurl/cacheurl.cc b/plugins/cacheurl/cacheurl.cc
>> index c32d52d..69feb06 100644
>> --- a/plugins/cacheurl/cacheurl.cc
>> +++ b/plugins/cacheurl/cacheurl.cc
>> @@ -403,7 +403,6 @@ TSRemapDeleteInstance(void *ih)
>>
>> TSDebug(PLUGIN_NAME, "Deleting remap instance");
>>
>> - TSfree(prl);
>> delete prl;
>> }
>
>
> This is unrelated, but what's wrong with just NULLing the
> pointers in our ats_free() functions?
>
> diff --git lib/ts/ink_memory.cc lib/ts/ink_memory.cc
> index 91aa403..2cca892 100644
> --- lib/ts/ink_memory.cc
> +++ lib/ts/ink_memory.cc
> @@ -119,23 +119,22 @@ ats_memalign(size_t alignment, size_t size)
> void
> ats_free(void *ptr)
> {
> - if (likely(ptr != NULL))
> - free(ptr);
> + ats_free_null(ptr);
> } /* End ats_free */
>
> void*
> ats_free_null(void *ptr)
> {
> - if (likely(ptr != NULL))
> - free(ptr);
> + free(ptr);
> + ptr = NULL;
> return NULL;
> } /* End ats_free_null */
>
> void
> ats_memalign_free(void *ptr)
> {
> - if (likely(ptr))
> - free(ptr);
> + free(ptr);
> + ptr = NULL;
Hmmm, I don't understand, what does setting ptr accomplish? The variable goes out of scope anyways.
-- Leif
> }
>
> // This effectively makes mallopt() a no-op (currently) when tcmalloc
>
>
> free(3) should be able to handle that NULL pointer all right by itself,
> and we are making sure to NULL it, such that potential reuse may be noticed.
>
> -- i
> Igor Galić
>
> Tel: +43 (0) 664 886 22 883
> Mail: i.galic@brainsware.org
> URL: http://brainsware.org/
> GPG: 8716 7A9F 989B ABD5 100F 4008 F266 55D6 2998 1641
>