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?
> +}
> +
> 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