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 <mchqwe...@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 <br...@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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Bryce


Regards,
Marek
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to