898338 - [binary data] formatting of binary data files is incorrect

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Niko Matsakis [:nmatsakis]]

While browsing the new Harmony binary data files, I realized that when reviewing I missed some obvious places where the formatting does not follow the SpiderMonkey conventions:

- Missing header, as njn pointed out on bug 578700:

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
 * vim: set ts=8 sts=4 et sw=4 tw=99:
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

- 2 space indents for public/private and 4 total for class members, so e.g. BinaryStruct should be as follows:

class BinaryStruct : public JSObject
{
  private:
    static JSObject *createEmpty(JSContext *cx, HandleObject type);
    static JSObject *create(JSContext *cx, HandleObject type);
  ...
}

- comment the close of the namespace at the end of BinaryData.h:

} // namespace js

Patch coming.

Comment 1

[Niko Matsakis [:nmatsakis]]

Attachment: bug898338-binary-data-formatting.diff

Primarily adjusts formatting.

Also, convert the static DataType etc definitions in BinaryData.h into extern definitions for the globals, define globals in BinaryData.cpp.

Comment 2

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

Comment on attachment 781610 [details] [diff] [review]
bug898338-binary-data-formatting.diff

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

::: js/src/builtin/BinaryData.cpp
@@ -791,2 @@
>          if (valueStr)
> -            valueChars = JS_EncodeString(cx, valueStr);

Was this removed accidentally?

Comment 3

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

Landed with the above line re-added, since it seems to be accidental. Everything else looks good.

https://hg.mozilla.org/integration/mozilla-inbound/rev/79322a7a9e09

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/79322a7a9e09