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