[pacman-dev] [PATCH] Add a helper macro to free() dload_payload structure

[Allan McRae]

On 27/4/20 12:24 pm, Anatol Pomozov wrote:
> Hi
> 
> On Sun, Apr 26, 2020 at 7:18 PM Allan McRae <allan at archlinux.org> wrote:
>>
>> On 27/4/20 11:54 am, Anatol Pomozov wrote:
>>> Hi
>>>
>>> On Sun, Apr 26, 2020 at 4:54 PM Allan McRae <allan at archlinux.org> wrote:
>>>>
>>>> On 24/4/20 2:40 pm, Anatol Pomozov wrote:
>>>>> It frees all the dynamically allocated fields plus the struct itself
>>>>>
>>>>
>>>> How many times will you use this?   Across how many functions?
>>>
>>> Currently my tree uses this macro 6 times. All of them in error
>>> codepath like this one:
>>>
>>> STRDUP(payload->fileurl, url,
>>>     DLOAD_PAYLOAD_FREE(payload); GOTO_ERR(handle, ALPM_ERR_MEMORY, err));
>>>
>>> An alternative to it is to inline DLOAD_PAYLOAD_FREE() macro into the
>>> STRDUP parameter.
>>> But it might look too verbose in this use-case:
>>>
>>> STRDUP(payload->fileurl, url,
>>>     _alpm_dload_payload_reset(payload);  FREE(payload);
>>>     GOTO_ERR(handle, ALPM_ERR_MEMORY, err));
>>>
>>
>> OK - that seems fine.
>>
>>>> We usually #unset defines not used globally too.
>>>
>>> I am not sure I understand this #unset requirement. Could you please
>>> give an example how it should work here?
>>>
>>
>> I was trying to establish of the usage of this define would be all in
>> one function.  Or is it needed in multiple.
>>
>> If it was in one function, we can #define it in the function and #undef
>> it at the end.  There are examples in libalpm/hook.c and util.c .
> 
> Currently this macro is used in 3 different files:
> 
> lib/libalpm/be_sync.c:192:
> DLOAD_PAYLOAD_FREE(payload); GOTO_ERR(handle, ALPM_ERR_MEMORY,
> cleanup));
> lib/libalpm/be_sync.c:208:
> DLOAD_PAYLOAD_FREE(sig_payload); GOTO_ERR(handle, ALPM_ERR_MEMORY,
> cleanup));
> lib/libalpm/dload.c:825:
> DLOAD_PAYLOAD_FREE(payload); GOTO_ERR(handle, ALPM_ERR_MEMORY, err));
> lib/libalpm/dload.h:59:#define DLOAD_PAYLOAD_FREE(payload) { \
> lib/libalpm/sync.c:741:
> DLOAD_PAYLOAD_FREE(payload); GOTO_ERR(handle, ALPM_ERR_MEMORY,
> finish));
> lib/libalpm/sync.c:743:
> DLOAD_PAYLOAD_FREE(payload); GOTO_ERR(handle, ALPM_ERR_MEMORY,
> finish));
> lib/libalpm/sync.c:766:
> DLOAD_PAYLOAD_FREE(sig_payload); GOTO_ERR(handle, ALPM_ERR_MEMORY,
> finish));
> 

Thanks - that is the context I needed.

Please make this a two line _alpm_dload_payload_free() function, instead
of a macro.  It will be optimised to the same thing by the compiler, and
comes with type safety etc.  Also, I'd prefer not to have macros outside
the utility functions in util.c.

Allan