Re: [PATCH wayland v2 2/2] scanner: get rid of leaks

2015-07-27 Thread Marek Chalupa



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 


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 

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_

Re: [PATCH wayland v2 2/2] scanner: get rid of leaks

2015-07-23 Thread Bryce Harrington
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 

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 

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

[PATCH wayland v2 2/2] scanner: get rid of leaks

2015-07-22 Thread Marek Chalupa
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 
---
 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'