Closed Bug 898338 Opened 7 years ago Closed 7 years ago

[binary data] formatting of binary data files is incorrect

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: nmatsakis, Assigned: nmatsakis)

References

Details

Attachments

(1 file)

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.
Primarily adjusts formatting.

Also, convert the static DataType etc definitions in BinaryData.h into extern definitions for the globals, define globals in BinaryData.cpp.
Assignee: general → nmatsakis
Attachment #781610 - Flags: review?(nsm.nikhil)
Summary: (harmony:bindata) formatting of binary data files is incorrect → formatting of binary data files is incorrect
Summary: formatting of binary data files is incorrect → [binary data] formatting of binary data files is incorrect
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?
Attachment #781610 - Flags: review?(nsm.nikhil) → review-
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
https://hg.mozilla.org/mozilla-central/rev/79322a7a9e09
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.