[v6.x] src: some refactoring of node_url.cc by addaleax · Pull Request #18324 · nodejs/node
added 5 commits
January 23, 2018 20:04- Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference PR-URL: nodejs#17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
This helps because `static` doesn’t work for C++ classes, but refactoring `url_host` into a proper C++ class seems the most reasonable soluation for the memory leak fixed by the next commit. PR-URL: nodejs#17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
- Gives `URLHost` a proper destructor that clears memory depending on the type of the host (This fixes a memory leak) - Hide the host type enums and class layout as implementation details - Make the `Parse` methods members of `URLHost` - Turn `WriteHost` into a `ToString()` method on the `URLHost` class - Verify that at the beginning of a parse attempt, the type is set to “failed” - Remove a lot of `goto`s from the source code 🐢🚀 PR-URL: nodejs#17470 Fixes: nodejs#17448 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
`last_piece` pointed to the end of the 8×16 bit array, so `piece_pointer == last_piece` already means that the pointer is not writable any longer. Previously, this still worked most of the time but could result in an out-of-bounds-write. Also, rename `last_piece` to `buffer_end` to avoid this pitfall. PR-URL: nodejs#17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 5, 2018- Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 5, 2018MylesBorins pushed a commit that referenced this pull request
Feb 5, 2018- Gives `URLHost` a proper destructor that clears memory depending on the type of the host (This fixes a memory leak) - Hide the host type enums and class layout as implementation details - Make the `Parse` methods members of `URLHost` - Turn `WriteHost` into a `ToString()` method on the `URLHost` class - Verify that at the beginning of a parse attempt, the type is set to “failed” - Remove a lot of `goto`s from the source code 🐢🚀 Backport-PR-URL: #18324 PR-URL: #17470 Fixes: #17448 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 5, 2018`last_piece` pointed to the end of the 8×16 bit array, so `piece_pointer == last_piece` already means that the pointer is not writable any longer. Previously, this still worked most of the time but could result in an out-of-bounds-write. Also, rename `last_piece` to `buffer_end` to avoid this pitfall. Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 5, 2018This would require C++11 features that would otherwise not be available on clang + OS X on Node 6.x. PR-URL: #18324 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 11, 2018- Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 11, 2018MylesBorins pushed a commit that referenced this pull request
Feb 11, 2018- Gives `URLHost` a proper destructor that clears memory depending on the type of the host (This fixes a memory leak) - Hide the host type enums and class layout as implementation details - Make the `Parse` methods members of `URLHost` - Turn `WriteHost` into a `ToString()` method on the `URLHost` class - Verify that at the beginning of a parse attempt, the type is set to “failed” - Remove a lot of `goto`s from the source code 🐢🚀 Backport-PR-URL: #18324 PR-URL: #17470 Fixes: #17448 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 11, 2018`last_piece` pointed to the end of the 8×16 bit array, so `piece_pointer == last_piece` already means that the pointer is not writable any longer. Previously, this still worked most of the time but could result in an out-of-bounds-write. Also, rename `last_piece` to `buffer_end` to avoid this pitfall. Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 11, 2018This would require C++11 features that would otherwise not be available on clang + OS X on Node 6.x. PR-URL: #18324 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 12, 2018- Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 12, 2018MylesBorins pushed a commit that referenced this pull request
Feb 12, 2018- Gives `URLHost` a proper destructor that clears memory depending on the type of the host (This fixes a memory leak) - Hide the host type enums and class layout as implementation details - Make the `Parse` methods members of `URLHost` - Turn `WriteHost` into a `ToString()` method on the `URLHost` class - Verify that at the beginning of a parse attempt, the type is set to “failed” - Remove a lot of `goto`s from the source code 🐢🚀 Backport-PR-URL: #18324 PR-URL: #17470 Fixes: #17448 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 12, 2018`last_piece` pointed to the end of the 8×16 bit array, so `piece_pointer == last_piece` already means that the pointer is not writable any longer. Previously, this still worked most of the time but could result in an out-of-bounds-write. Also, rename `last_piece` to `buffer_end` to avoid this pitfall. Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 12, 2018This would require C++11 features that would otherwise not be available on clang + OS X on Node 6.x. PR-URL: #18324 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 13, 2018- Remove pointless pointers - Make `WriteHost` take a const argument so that it’s functionality is clear from the signature - Make `FindLongestZeroSequence` templated to accommodate the constness in `WriteHost` and because using `uint16_t` is an articifial, unnecessary restriction - Remove string copying when no copies are needed - Make `PercentDecode` just return its return value - Make `ParseHost` (string-only version) take its constant argument as a constant reference Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 13, 2018MylesBorins pushed a commit that referenced this pull request
Feb 13, 2018- Gives `URLHost` a proper destructor that clears memory depending on the type of the host (This fixes a memory leak) - Hide the host type enums and class layout as implementation details - Make the `Parse` methods members of `URLHost` - Turn `WriteHost` into a `ToString()` method on the `URLHost` class - Verify that at the beginning of a parse attempt, the type is set to “failed” - Remove a lot of `goto`s from the source code 🐢🚀 Backport-PR-URL: #18324 PR-URL: #17470 Fixes: #17448 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 13, 2018`last_piece` pointed to the end of the 8×16 bit array, so `piece_pointer == last_piece` already means that the pointer is not writable any longer. Previously, this still worked most of the time but could result in an out-of-bounds-write. Also, rename `last_piece` to `buffer_end` to avoid this pitfall. Backport-PR-URL: #18324 PR-URL: #17470 Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request
Feb 13, 2018This would require C++11 features that would otherwise not be available on clang + OS X on Node 6.x. PR-URL: #18324 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters