[PATCH wayland v2 2/2] scanner: get rid of leaks
Marek Chalupa
mchqwerty at gmail.com
Mon Jul 27 00:26:32 PDT 2015
On 07/23/2015 08:50 PM, Bryce Harrington wrote:
> On Thu, Jul 23, 2015 at 07:39:31AM +0200, Marek Chalupa wrote:
>> Free all the memory we have allocated during running.
>>
>> v2.: split creating objects and getting rid of leaks
>> into two patches
>>
>> move check for NULL description into free_description
>>
>> Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
>
> This one looks good but looks like it depends on patch #1 in the series
> so will likely need rebased once that one is fixed. This one looks fine
> though, so:
>
> Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
>
> One question at the very end...
>
>> ---
>> src/scanner.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 105 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index 85c283b..92f336f 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -333,6 +333,15 @@ create_message(struct location loc, const char *name)
>> return message;
>> }
>>
>> +static void
>> +free_arg(struct arg *arg)
>> +{
>> + free(arg->name);
>> + free(arg->interface_name);
>> + free(arg->summary);
>> + free(arg);
>> +}
>> +
>> static struct arg *
>> create_arg(const char *name, const char *type)
>> {
>> @@ -359,12 +368,41 @@ create_arg(const char *name, const char *type)
>> arg->type = NEW_ID;
>> else if (strcmp(type, "object") == 0)
>> arg->type = OBJECT;
>> - else
>> + else {
>> + free_arg(arg);
>> return NULL;
>> + }
>>
>> return arg;
>> }
>>
>> +static void
>> +free_description(struct description *desc)
>> +{
>> + if (!desc)
>> + return;
>> +
>> + free(desc->summary);
>> + free(desc->text);
>> +
>> + free(desc);
>> +}
>> +
>> +static void
>> +free_message(struct message *message)
>> +{
>> + struct arg *a, *a_next;
>> +
>> + free(message->name);
>> + free(message->uppercase_name);
>> + free_description(message->description);
>> +
>> + wl_list_for_each_safe(a, a_next, &message->arg_list, link)
>> + free_arg(a);
>> +
>> + free(message);
>> +}
>> +
>> static struct enumeration *
>> create_enumeration(const char *name)
>> {
>> @@ -393,6 +431,32 @@ create_entry(const char *name, const char *value)
>> return entry;
>> }
>>
>> +static void
>> +free_entry(struct entry *entry)
>> +{
>> + free(entry->name);
>> + free(entry->uppercase_name);
>> + free(entry->value);
>> + free(entry->summary);
>> +
>> + free(entry);
>> +}
>> +
>> +static void
>> +free_enumeration(struct enumeration *enumeration)
>> +{
>> + struct entry *e, *e_next;
>> +
>> + free(enumeration->name);
>> + free(enumeration->uppercase_name);
>> + free_description(enumeration->description);
>> +
>> + wl_list_for_each_safe(e, e_next, &enumeration->entry_list, link)
>> + free_entry(e);
>> +
>> + free(enumeration);
>> +}
>> +
>> static struct interface *
>> create_interface(struct location loc, const char *name, int version)
>> {
>> @@ -413,6 +477,26 @@ create_interface(struct location loc, const char *name, int version)
>> }
>>
>> static void
>> +free_interface(struct interface *interface)
>> +{
>> + struct message *m, *next_m;
>> + struct enumeration *e, *next_e;
>> +
>> + free(interface->name);
>> + free(interface->uppercase_name);
>> + free_description(interface->description);
>> +
>> + wl_list_for_each_safe(m, next_m, &interface->request_list, link)
>> + free_message(m);
>> + wl_list_for_each_safe(m, next_m, &interface->event_list, link)
>> + free_message(m);
>> + wl_list_for_each_safe(e, next_e, &interface->enumeration_list, link)
>> + free_enumeration(e);
>> +
>> + free(interface);
>> +}
>> +
>> +static void
>> start_element(void *data, const char *element_name, const char **atts)
>> {
>> struct parse_context *ctx = data;
>> @@ -1101,7 +1185,7 @@ get_include_name(bool core, enum side side)
>> static void
>> emit_header(struct protocol *protocol, enum side side)
>> {
>> - struct interface *i;
>> + struct interface *i, *i_next;
>> struct wl_array types;
>> const char *s = (side == SERVER) ? "SERVER" : "CLIENT";
>> char **p, *prev;
>> @@ -1154,7 +1238,7 @@ emit_header(struct protocol *protocol, enum side side)
>>
>> printf("\n");
>>
>> - wl_list_for_each(i, &protocol->interface_list, link) {
>> + wl_list_for_each_safe(i, i_next, &protocol->interface_list, link) {
>>
>> emit_enumerations(i);
>>
>> @@ -1168,6 +1252,8 @@ emit_header(struct protocol *protocol, enum side side)
>> emit_opcodes(&i->request_list, i);
>> emit_stubs(&i->request_list, i);
>> }
>> +
>> + free_interface(i);
>> }
>>
>> printf("#ifdef __cplusplus\n"
>> @@ -1283,7 +1369,7 @@ emit_messages(struct wl_list *message_list,
>> static void
>> emit_code(struct protocol *protocol)
>> {
>> - struct interface *i;
>> + struct interface *i, *next;
>> struct wl_array types;
>> char **p, *prev;
>>
>> @@ -1318,7 +1404,7 @@ emit_code(struct protocol *protocol)
>> }
>> printf("};\n\n");
>>
>> - wl_list_for_each(i, &protocol->interface_list, link) {
>> + wl_list_for_each_safe(i, next, &protocol->interface_list, link) {
>>
>> emit_messages(&i->request_list, i, "requests");
>> emit_messages(&i->event_list, i, "events");
>> @@ -1341,9 +1427,21 @@ emit_code(struct protocol *protocol)
>> printf("\t0, NULL,\n");
>>
>> printf("};\n\n");
>> +
>> + /* we won't need it any further */
>> + free_interface(i);
>> }
>> }
>>
>> +static void
>> +free_protocol(struct protocol *protocol)
>> +{
>> + free(protocol->name);
>> + free(protocol->uppercase_name);
>> + free(protocol->copyright);
>> + free_description(protocol->description);
>
> In all the other structures I get that we don't need to do anything with
> the 'link' field since those are just references to already managed
> memory, but I'm curious in the protocol structure if it's the same case
> with interface_list? Or do we need to also free the contents of it?
The contents of interface_list is freed by the free_interface(), so we
should not free it here again (we do not remove an interface from the
list after freeing, but since nothing takes action on the list later, it
should be fine).
>
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> struct parse_context ctx;
>> @@ -1467,5 +1565,7 @@ int main(int argc, char *argv[])
>> break;
>> }
>>
>> + free_protocol(&protocol);
>> +
>> return 0;
>> }
>> --
>> 2.4.3
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
> Bryce
>
Regards,
Marek
More information about the wayland-devel
mailing list