Hi On Tue, Apr 28, 2020 at 5:56 PM Allan McRae <[email protected]> wrote: > > On 27/4/20 12:24 pm, Anatol Pomozov wrote: > > Hi > > > > On Sun, Apr 26, 2020 at 7:18 PM Allan McRae <[email protected]> wrote: > >> > >> On 27/4/20 11:54 am, Anatol Pomozov wrote: > >>> Hi > >>> > >>> On Sun, Apr 26, 2020 at 4:54 PM Allan McRae <[email protected]> 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.
Sure I can make it a function. Though an advantage of using macro here is that it sets the variable to NULL the same way as FREE() macro does. I found such defensive programming technique useful.
