Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: fix StringBytes::Write() #1042

Merged
merged 8 commits into from
Mar 5, 2015
Merged

src: fix StringBytes::Write() #1042

merged 8 commits into from
Mar 5, 2015

Commits on Mar 5, 2015

  1. src: remove unused code 

    PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    f5b7e18 View commit details
    Browse the repository at this point in the history

  2. src: fix gc heuristic for external twobyte strings 

    Large external two-byte strings reported their character length instead
of their byte length, throwing off the garbage collector heuristic by
a factor of two.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    826cde8 View commit details
    Browse the repository at this point in the history

  3. src: remove NODE_INVALID_UTF8 environment variable 

    Introduced in joyent/node v0.10 as a backwards compatibility measure.
It's an ugly hack and allowing invalid UTF-8 is not a good idea in the
first place, remove it.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    364cc7e View commit details
    Browse the repository at this point in the history

  4. src: simplify node::Utf8Value() 

    * Remove kStorageSize constant.

* Remove superfluous local variable and reinterpret_cast.

* Reorder data members so the length field and data pointer
  (if not the data itself) fit in a single cache line.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    c9ee654 View commit details
    Browse the repository at this point in the history

  5. src: rename confusingly named local variable 

    Rename `val_` to `string`.  The underscore suffix is normally reserved
for data members, not locals, and it's not a great name in the first
place.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    4aea16f View commit details
    Browse the repository at this point in the history

  6. test: simplify parallel/test-stringbytes-external 

    Make the algorithm that creates the big input strings a little easier
to comprehend.  No functional changes, the string lengths are unchanged.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    e2fb733 View commit details
    Browse the repository at this point in the history

  7. src: fix external string length calculation 

    Make StringBytes::GetExternalParts() return the byte length for two-byte
strings, not the character length.  Its callers operate on bytes, not
characters.

This also fixes StringBytes::Size() reporting only half of the actual
number of bytes for external two-byte strings.

PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    2eda2d6 View commit details
    Browse the repository at this point in the history

  8. src: fix ucs-2 buffer encoding regression 

    StringBytes::Write() did a plain memcpy() when is_extern is true but
that's wrong when the source is a two-byte string and the destination
a one-byte or UTF-8 string.

The impact is limited to strings > 1,031,913 bytes because those are
normally the only strings that are externalized, although the use of
the 'externalize strings' extension (--expose_externalize_string) can
also trigger it.

This commit also cleans up the bytes versus characters confusion in
StringBytes::Write() because that was closely intertwined with the
UCS-2 encoding regression.  One wasn't fixable without the other.

Fixes: nodejs#1024
Fixes: nodejs/node-v0.x-archive#8683
PR-URL: nodejs#1042
Reviewed-By: Trevor Norris <[email protected]>
    @bnoordhuis
    bnoordhuis committed Mar 5, 2015
    Configuration menu
    Copy the full SHA
    1640ded View commit details
    Browse the repository at this point in the history