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

Allan McRae allan at archlinux.org
Wed Apr 29 00:55:53 UTC 2020


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


More information about the pacman-dev mailing list