898675 - [binary data] FieldList is not freed on error in StructType::layout

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

errors in layout (places where layout() returns false) should free the FieldList that is allocated.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Binary Data: Free StructType fieldList on error.

Comment 2

[Niko Matsakis [:nmatsakis]]

Comment on attachment 782270 [details] [diff] [review]
Binary Data: Free StructType fieldList on error.

Review of attachment 782270 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/BinaryData.cpp
@@ +1528,4 @@
>  
>          RootedObject fieldType(cx, fieldTypeVal.toObjectOrNull());
>          if (!IsBinaryType(fieldType))
> +            goto error;

I think we should report some sort of error here.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

The error is reported by StructType::create(), should it be more specific? Landing for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0da6dd571263

Comment 4

[Niko Matsakis [:nmatsakis]]

Ah, I see. Usually the convention as I understand it is that if a function takes a JSContext *cx, it will ensure that an error is reported if it returns an error. Maybe just add a comment.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0da6dd571263