-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Conversation
The -fno-strict-aliasing flag was added to fix compilation warnings when building Node.js with GCC <= 4.4 Reviewed-By: Julien Gilli <[email protected]> PR-URL: #9179 PR: #25141 PR-URL: #25141 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Julien Gilli <[email protected]>
Make that 1.6.1 :-) |
@@ -225,11 +225,10 @@ int uv_fs_event_start(uv_fs_event_t* handle, | |||
} | |||
|
|||
if (!handle->buffer) { | |||
handle->buffer = (char*)_aligned_malloc(uv_directory_watcher_buffer_size, | |||
sizeof(DWORD)); | |||
handle->buffer = (char*)uv__malloc(uv_directory_watcher_buffer_size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saghul IIUC, here the call to _aligned_malloc
would be replaced (by default) by a call to malloc
, is it intentional and what is the impact of such a change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was reviewed back in the day and Bert pointed out that plain malloc
provided the necessary alignment in that case, that's why we dropped it.
On Jun 11, 2015 21:05, "Julien Gilli" [email protected] wrote:
In deps/uv/src/win/fs-event.c
#25475 (comment):@@ -225,11 +225,10 @@ int uv_fs_event_start(uv_fs_event_t* handle,
}if (!handle->buffer) {
- handle->buffer = (char*)_aligned_malloc(uv_directory_watcher_buffer_size,
sizeof(DWORD));
- handle->buffer = (char*)uv__malloc(uv_directory_watcher_buffer_size);
@saghul https://github.com/saghul IIUC, here the call to _aligned_malloc
would be replaced by a call to malloc, is it intentional and what is the
impact of such a change?—
Reply to this email directly or view it on GitHub
https://github.com/joyent/node/pull/25475/files#r32255959.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saghul Sounds good, thank you for the clarification!
@saghul One question just to make sure I don't leave any stone unturned, otherwise that looks good. Will land asap unless the answer to that question means we need to change something. Thank you once again @saghul 👍 |
LGTM. |
@saghul @joyent/node-collaborators Landing with http://jenkins.nodejs.org/job/node-accept-pull-request/127/. |
Fixes: #9310 Reviewed-By: Julien Gilli <[email protected]> PR-URL: #25475
The -fno-strict-aliasing flag was added to fix compilation warnings when building Node.js with GCC <= 4.4 PR: #25141 Reviewed-By: Julien Gilli <[email protected]> PR-URL: #25475
R=@misterdjules