8:01 a.m.
While coding my latest patch I found an annoying const warning when
freeing strings we allocate with talloc but then assign to a variable
of type "const char *".
This made me investigate our talloc_zfree macro and I found a weird
patch, made back in the times that really doesn't make much sense.
I reverted that and then applied a fix to avoid const warnings, as they
are bogus in this case.
I also removed a duplicated macro that was out of sync already.
Simo Sorce (3):
Revert "Avoid accessing half-deallocated memory when using
talloc_zfree macro."
Avoid duplicating macros
Avoid const warnings when deallocating memory
src/ldb_modules/memberof.c | 4 ----
src/util/util.h | 6 +-----
2 files changed, 1 insertions(+), 9 deletions(-)
8:01 a.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
This commit doesn't really make sense, we are never accessing freed
memory as all we are dealing with is a pointer which is never itsef
part of the memory we are freeing (if it were, it would be an error
in the caller and we shouldn't mask it in this macro).
---
src/util/util.h | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { \
- TALLOC_CTX *_tmp_ctx = ptr; \
- ptr = NULL; \
- talloc_free(_tmp_ctx); \
- } while(0)
+#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
#endif
#ifndef discard_const_p
--
1.7.1
8:01 a.m.
New subject: [PATCH 2/3] Avoid duplicating macros
This macro is already available in util/util.h which is expicitly included
in this file.
---
src/ldb_modules/memberof.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
index f0b5b72ed11ce71aec8ccb6e7a7cbd2d02e9566e..d0fe7349238ee91ba48f2ccd78516f43ab31a8e4
100644
--- a/src/ldb_modules/memberof.c
+++ b/src/ldb_modules/memberof.c
@@ -35,10 +35,6 @@
#define DB_USER_CLASS "user"
#define DB_OC "objectClass"
-#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
-#endif
-
#ifndef MAX
#define MAX(a,b) (((a) > (b)) ? (a) : (b))
#endif
--
1.7.1
8:01 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
In some case we allocate and assign data to a const pointer.
When we then try to free it we would get a const warning because talloc_free
accepts a void, not a const void pointer. Use discard_const to avoid the
warning, it is safe in this case.
---
src/util/util.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index 66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
+#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; } while(0)
#endif
#ifndef discard_const_p
--
1.7.1
10:16 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On Wed, Nov 14, 2012 at 09:01:15AM -0500, Simo Sorce wrote:
In some case we allocate and assign data to a const pointer.
When we then try to free it we would get a const warning because talloc_free
accepts a void, not a const void pointer. Use discard_const to avoid the
warning, it is safe in this case.
---
src/util/util.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index 66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
+#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; } while(0)
I wonder if it would be cleaner to add a talloc_zfree_const() or similar
which can be used in places where it is safe to suppress the warning.
At other place the warning might be useful to discover e.g. some
copy-and-paste errors.
bye,
Sumit
#endif
#ifndef discard_const_p
--
1.7.1
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
10:58 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On Wed, 2012-11-14 at 17:16 +0100, Sumit Bose wrote:
On Wed, Nov 14, 2012 at 09:01:15AM -0500, Simo Sorce wrote:
> In some case we allocate and assign data to a const pointer.
> When we then try to free it we would get a const warning because talloc_free
> accepts a void, not a const void pointer. Use discard_const to avoid the
> warning, it is safe in this case.
> ---
> src/util/util.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/util.h b/src/util/util.h
> index
66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
> #define FLAGS_PID_FILE 0x0004
>
> #ifndef talloc_zfree
> -#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
> +#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; }
while(0)
I wonder if it would be cleaner to add a talloc_zfree_const() or similar
which can be used in places where it is safe to suppress the warning.
At other place the warning might be useful to discover e.g. some
copy-and-paste errors.
No, I thought about it but I do not think it is worth the complexity.
If you free a truly const (ie not allocated) piece of memory you'll get
a talloc error/abort which is sufficient I think :)
Simo.
--
Simo Sorce * Red Hat, Inc * New York
9:51 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On 11/14/2012 03:01 PM, Simo Sorce wrote:
In some case we allocate and assign data to a const pointer.
When we then try to free it we would get a const warning because talloc_free
accepts a void, not a const void pointer. Use discard_const to avoid the
warning, it is safe in this case.
---
src/util/util.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index 66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
+#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; } while(0)
#endif
#ifndef discard_const_p
Hi,
I don't see any warnings without this patch on F17 with
libtalloc-devel-2.0.7-4.fc17.x86_64. Does this patch apply to specific
version?
10:04 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On Mon, 2012-11-26 at 16:51 +0100, Pavel Březina wrote:
On 11/14/2012 03:01 PM, Simo Sorce wrote:
> In some case we allocate and assign data to a const pointer.
> When we then try to free it we would get a const warning because talloc_free
> accepts a void, not a const void pointer. Use discard_const to avoid the
> warning, it is safe in this case.
> ---
> src/util/util.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/util.h b/src/util/util.h
> index
66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
> #define FLAGS_PID_FILE 0x0004
>
> #ifndef talloc_zfree
> -#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
> +#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; }
while(0)
> #endif
>
> #ifndef discard_const_p
>
Hi,
I don't see any warnings without this patch on F17 with
libtalloc-devel-2.0.7-4.fc17.x86_64. Does this patch apply to specific
version?
I t was needed at some point in one of the patches I recently sent to
the list, however I have since changed the code (after Sumit's request)
so it is not strictly needed. However I think it make sense to avoid
errors here because freeing an allocated const char * string is legit.
I see however the argument that maybe we normally shouldn't declare
allocated strings as const, but there may be cases where this is
actually useful.
Do you see any downside in discarding const in talloc_zfree() ?
Simo.
--
Simo Sorce * Red Hat, Inc * New York
10:52 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On 11/26/2012 05:04 PM, Simo Sorce wrote:
On Mon, 2012-11-26 at 16:51 +0100, Pavel Březina wrote:
> On 11/14/2012 03:01 PM, Simo Sorce wrote:
>> In some case we allocate and assign data to a const pointer.
>> When we then try to free it we would get a const warning because talloc_free
>> accepts a void, not a const void pointer. Use discard_const to avoid the
>> warning, it is safe in this case.
>> ---
>> src/util/util.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/util.h b/src/util/util.h
>> index
66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
>> #define FLAGS_PID_FILE 0x0004
>>
>> #ifndef talloc_zfree
>> -#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
>> +#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL; }
while(0)
>> #endif
>>
>> #ifndef discard_const_p
>>
>
> Hi,
> I don't see any warnings without this patch on F17 with
> libtalloc-devel-2.0.7-4.fc17.x86_64. Does this patch apply to specific
> version?
I t was needed at some point in one of the patches I recently sent to
the list, however I have since changed the code (after Sumit's request)
so it is not strictly needed. However I think it make sense to avoid
errors here because freeing an allocated const char * string is legit.
I see however the argument that maybe we normally shouldn't declare
allocated strings as const, but there may be cases where this is
actually useful.
Do you see any downside in discarding const in talloc_zfree() ?
As you wrote earlier to Sumit questions, if you try to talloc_free
non-heap memory, talloc will tell you. However, if this patch is not
needed right now, I prefer compiler warning.
In general, I think freeing const variable means that something is wrong
with design, although I found myself in this situation once or twice (in
other projects).
But I'm giving you at least ack to the code. :-)
1:07 p.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On Mon, 2012-11-26 at 17:52 +0100, Pavel Březina wrote:
On 11/26/2012 05:04 PM, Simo Sorce wrote:
> On Mon, 2012-11-26 at 16:51 +0100, Pavel Březina wrote:
>> On 11/14/2012 03:01 PM, Simo Sorce wrote:
>>> In some case we allocate and assign data to a const pointer.
>>> When we then try to free it we would get a const warning because
talloc_free
>>> accepts a void, not a const void pointer. Use discard_const to avoid the
>>> warning, it is safe in this case.
>>> ---
>>> src/util/util.h | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/util/util.h b/src/util/util.h
>>> index
66bae505df2779dbfc065a09162fd789fef9e835..e85facb92bb4de8d9eeaf97716e1aac252f9b32c 100644
>>> --- a/src/util/util.h
>>> +++ b/src/util/util.h
>>> @@ -228,7 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
>>> #define FLAGS_PID_FILE 0x0004
>>>
>>> #ifndef talloc_zfree
>>> -#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
>>> +#define talloc_zfree(ptr) do { talloc_free(discard_const(ptr)); ptr = NULL;
} while(0)
>>> #endif
>>>
>>> #ifndef discard_const_p
>>>
>>
>> Hi,
>> I don't see any warnings without this patch on F17 with
>> libtalloc-devel-2.0.7-4.fc17.x86_64. Does this patch apply to specific
>> version?
>
> I t was needed at some point in one of the patches I recently sent to
> the list, however I have since changed the code (after Sumit's request)
> so it is not strictly needed. However I think it make sense to avoid
> errors here because freeing an allocated const char * string is legit.
>
> I see however the argument that maybe we normally shouldn't declare
> allocated strings as const, but there may be cases where this is
> actually useful.
>
> Do you see any downside in discarding const in talloc_zfree() ?
As you wrote earlier to Sumit questions, if you try to talloc_free
non-heap memory, talloc will tell you. However, if this patch is not
needed right now, I prefer compiler warning.
In general, I think freeing const variable means that something is wrong
with design, although I found myself in this situation once or twice (in
other projects).
But I'm giving you at least ack to the code. :-)
Ok then I will wait for someone else to chime in whether we want this
patch in or not.
I do not have a strong preference either way.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
3:45 a.m.
New subject: [PATCH 3/3] Avoid const warnings when deallocating memory
On Mon, Nov 26, 2012 at 05:52:53PM +0100, Pavel Březina wrote:
In general, I think freeing const variable means that something is
wrong with design, although I found myself in this situation once or
twice (in other projects).
I don't think this is necessarily true; we often use a construct such
as:
const char *str;
str = talloc_asprintf("%s%s\n", foo, bar);
/* do stuff while treating str as const */
talloc_free(str);
The free above would produce a warning with glibc's free, but I think
if talloc discarded const here, we might get code that even treats str
as const, but is able to free str.
To me, the patch makes sense.
9:46 a.m.
New subject: [PATCH 2/3] Avoid duplicating macros
On 11/14/2012 03:01 PM, Simo Sorce wrote:
This macro is already available in util/util.h which is expicitly
included
in this file.
---
src/ldb_modules/memberof.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/src/ldb_modules/memberof.c b/src/ldb_modules/memberof.c
index f0b5b72ed11ce71aec8ccb6e7a7cbd2d02e9566e..d0fe7349238ee91ba48f2ccd78516f43ab31a8e4
100644
--- a/src/ldb_modules/memberof.c
+++ b/src/ldb_modules/memberof.c
@@ -35,10 +35,6 @@
#define DB_USER_CLASS "user"
#define DB_OC "objectClass"
-#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
-#endif
-
#ifndef MAX
#define MAX(a,b) (((a) > (b)) ? (a) : (b))
#endif
Ack.
10:01 a.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On 11/14/2012 03:01 PM, Simo Sorce wrote:
This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
This commit doesn't really make sense, we are never accessing freed
memory as all we are dealing with is a pointer which is never itsef
part of the memory we are freeing (if it were, it would be an error
in the caller and we shouldn't mask it in this macro).
---
src/util/util.h | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { \
- TALLOC_CTX *_tmp_ctx = ptr; \
- ptr = NULL; \
- talloc_free(_tmp_ctx); \
- } while(0)
+#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
#endif
#ifndef discard_const_p
Hi, we can use TALLOC_FREE(ctx) here, which comes from talloc.h and is
identical to this macro.
10:06 a.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On Wed, Nov 14, 2012 at 05:01:34PM +0100, Pavel Březina wrote:
On 11/14/2012 03:01 PM, Simo Sorce wrote:
>This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
>
>This commit doesn't really make sense, we are never accessing freed
>memory as all we are dealing with is a pointer which is never itsef
>part of the memory we are freeing (if it were, it would be an error
>in the caller and we shouldn't mask it in this macro).
>---
> src/util/util.h | 6 +-----
> 1 files changed, 1 insertions(+), 5 deletions(-)
>
>diff --git a/src/util/util.h b/src/util/util.h
>index
b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835 100644
>--- a/src/util/util.h
>+++ b/src/util/util.h
>@@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
> #define FLAGS_PID_FILE 0x0004
>
> #ifndef talloc_zfree
>-#define talloc_zfree(ptr) do { \
>- TALLOC_CTX *_tmp_ctx = ptr; \
>- ptr = NULL; \
>- talloc_free(_tmp_ctx); \
>- } while(0)
>+#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
> #endif
>
> #ifndef discard_const_p
Hi, we can use TALLOC_FREE(ctx) here, which comes from talloc.h and
is identical to this macro.
Is it present even in very old talloc versions? We still support RHEL5..
10:55 a.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On Wed, 2012-11-14 at 17:06 +0100, Jakub Hrozek wrote:
On Wed, Nov 14, 2012 at 05:01:34PM +0100, Pavel Březina wrote:
> On 11/14/2012 03:01 PM, Simo Sorce wrote:
> >This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
> >
> >This commit doesn't really make sense, we are never accessing freed
> >memory as all we are dealing with is a pointer which is never itsef
> >part of the memory we are freeing (if it were, it would be an error
> >in the caller and we shouldn't mask it in this macro).
> >---
> > src/util/util.h | 6 +-----
> > 1 files changed, 1 insertions(+), 5 deletions(-)
> >
> >diff --git a/src/util/util.h b/src/util/util.h
> >index
b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835 100644
> >--- a/src/util/util.h
> >+++ b/src/util/util.h
> >@@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
> > #define FLAGS_PID_FILE 0x0004
> >
> > #ifndef talloc_zfree
> >-#define talloc_zfree(ptr) do { \
> >- TALLOC_CTX *_tmp_ctx = ptr; \
> >- ptr = NULL; \
> >- talloc_free(_tmp_ctx); \
> >- } while(0)
> >+#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
> > #endif
> >
> > #ifndef discard_const_p
>
> Hi, we can use TALLOC_FREE(ctx) here, which comes from talloc.h and
> is identical to this macro.
Is it present even in very old talloc versions? We still support RHEL5..
It probably is now, but it is very ugly and we standardized our code to
use talloc_zfree(), I do not think it makes sense to change it.
plus TALLOC_FREE() does not discard const. Which is what I add in the
folloowing pathc, so we'd be back with our own macro.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
12:35 p.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On 11/14/2012 11:06 AM, Jakub Hrozek wrote:
On Wed, Nov 14, 2012 at 05:01:34PM +0100, Pavel Březina wrote:
> On 11/14/2012 03:01 PM, Simo Sorce wrote:
>> This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
>>
>> This commit doesn't really make sense, we are never accessing freed
>> memory as all we are dealing with is a pointer which is never itsef
>> part of the memory we are freeing (if it were, it would be an error
>> in the caller and we shouldn't mask it in this macro).
>> ---
>> src/util/util.h | 6 +-----
>> 1 files changed, 1 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/util/util.h b/src/util/util.h
>> index
b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
>> #define FLAGS_PID_FILE 0x0004
>>
>> #ifndef talloc_zfree
>> -#define talloc_zfree(ptr) do { \
>> - TALLOC_CTX *_tmp_ctx = ptr; \
>> - ptr = NULL; \
>> - talloc_free(_tmp_ctx); \
>> - } while(0)
>> +#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
>> #endif
>>
>> #ifndef discard_const_p
> Hi, we can use TALLOC_FREE(ctx) here, which comes from talloc.h and
> is identical to this macro.
Is it present even in very old talloc versions? We still support RHEL5..
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Why this is a concern?
We do not plan to support any of the latest versions on RHEL5. It is
less and less needed as we move forward.
I think it would be safe to say that 1.9 was the last version we build
for RHEL5.
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
2:07 p.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On Wed 14 Nov 2012 01:35:21 PM EST, Dmitri Pal wrote:
On 11/14/2012 11:06 AM, Jakub Hrozek wrote:
> On Wed, Nov 14, 2012 at 05:01:34PM +0100, Pavel Březina wrote:
>> On 11/14/2012 03:01 PM, Simo Sorce wrote:
>>> This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
>>>
>>> This commit doesn't really make sense, we are never accessing freed
>>> memory as all we are dealing with is a pointer which is never itsef
>>> part of the memory we are freeing (if it were, it would be an error
>>> in the caller and we shouldn't mask it in this macro).
>>> ---
>>> src/util/util.h | 6 +-----
>>> 1 files changed, 1 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/util/util.h b/src/util/util.h
>>> index
b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835 100644
>>> --- a/src/util/util.h
>>> +++ b/src/util/util.h
>>> @@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
>>> #define FLAGS_PID_FILE 0x0004
>>>
>>> #ifndef talloc_zfree
>>> -#define talloc_zfree(ptr) do { \
>>> - TALLOC_CTX *_tmp_ctx = ptr; \
>>> - ptr = NULL; \
>>> - talloc_free(_tmp_ctx); \
>>> - } while(0)
>>> +#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
>>> #endif
>>>
>>> #ifndef discard_const_p
>> Hi, we can use TALLOC_FREE(ctx) here, which comes from talloc.h and
>> is identical to this macro.
> Is it present even in very old talloc versions? We still support RHEL5..
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Why this is a concern?
We do not plan to support any of the latest versions on RHEL5. It is
less and less needed as we move forward.
I think it would be safe to say that 1.9 was the last version we build
for RHEL5.
It's also irrelevant, since RHEL carries libtalloc 2.0.x, which is
recent enough.
9:45 a.m.
New subject: [PATCH 1/3] Revert "Avoid accessing half-deallocated memory when using talloc_zfree macro."
On 11/14/2012 03:01 PM, Simo Sorce wrote:
This reverts commit ff57c6aeb80a52b1f52bd1dac9308a69dc7a4774.
This commit doesn't really make sense, we are never accessing freed
memory as all we are dealing with is a pointer which is never itsef
part of the memory we are freeing (if it were, it would be an error
in the caller and we shouldn't mask it in this macro).
---
src/util/util.h | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/src/util/util.h b/src/util/util.h
index b6ecfc2c952bd6a37f49194d961b3f762c9546e7..66bae505df2779dbfc065a09162fd789fef9e835
100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -228,11 +228,7 @@ errno_t set_debug_file_from_fd(const int fd);
#define FLAGS_PID_FILE 0x0004
#ifndef talloc_zfree
-#define talloc_zfree(ptr) do { \
- TALLOC_CTX *_tmp_ctx = ptr; \
- ptr = NULL; \
- talloc_free(_tmp_ctx); \
- } while(0)
+#define talloc_zfree(ptr) do { talloc_free(ptr); ptr = NULL; } while(0)
#endif
#ifndef discard_const_p
Ack.
4160
days inactive
4174
days old
sssd-devel@lists.fedorahosted.org
21 comments
6 participants
participants (6)
-
Dmitri Pal -
Jakub Hrozek -
Pavel Březina -
Simo Sorce -
Stephen Gallagher -
Sumit Bose