You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/02/20 12:17:12 UTC
[GitHub] [hadoop] GauthamBanasandra opened a new pull request #2710: HDFS-15843. Make write cross-platform
GauthamBanasandra opened a new pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710
* Currently write from unistd.h is used
which isn't cross-platform.
* We need to use std::cout.write instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] smengcl commented on pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
smengcl commented on pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#issuecomment-783214670
Thanks @GauthamBanasandra for the patch.
I'm wondering if the [author](https://issues.apache.org/jira/browse/HDFS-11028) uses syscall write for a reason. According to the comment, heap could be corrupted if the `printf` call uses `malloc` or equivalent (depending on C/C++ library's implementation):
https://github.com/apache/hadoop/pull/2710/files#diff-d2644a26b4354d1adbaacb60a244b47dbbebcef5cb4223ee52542d253f9a32a8R42
https://github.com/apache/hadoop/pull/2710/files#diff-e91e8d0a790404522dd7fc26faa7a89336b6171262f5ebdc18b6c43ec84adbb5L48
How about wrapping the `<unistd.h>` header import line and the existing approach in `#ifndef WINDOWS`? And use `#else` to wrap the new approach. Assuming Windows is the platform you are aiming for.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] goiri merged pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
goiri merged pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] GauthamBanasandra commented on pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#issuecomment-782794471
@smengcl could you please review this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] GauthamBanasandra commented on a change in pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on a change in pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#discussion_r584322501
##########
File path: hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/c/connect_cancel/connect_cancel.c
##########
@@ -43,10 +42,10 @@ const char *catch_exit = "Exiting the signal handler.\n";
// Print to stdout without calling malloc or otherwise indirectly modify userspace state.
// Write calls to stdout may still interleave with stuff coming from elsewhere.
static void sighandler_direct_stdout(const char *msg) {
- if(!msg)
+ if(!msg) {
return;
- ssize_t res = write(1 /*posix stdout fd*/, msg, strlen(msg));
- (void)res;
+ }
+ printf("%s", msg);
Review comment:
I've removed this. Please take a look at the latest commit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] GauthamBanasandra commented on a change in pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on a change in pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#discussion_r580055486
##########
File path: hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/c/connect_cancel/connect_cancel.c
##########
@@ -43,10 +42,10 @@ const char *catch_exit = "Exiting the signal handler.\n";
// Print to stdout without calling malloc or otherwise indirectly modify userspace state.
// Write calls to stdout may still interleave with stuff coming from elsewhere.
static void sighandler_direct_stdout(const char *msg) {
- if(!msg)
+ if(!msg) {
return;
- ssize_t res = write(1 /*posix stdout fd*/, msg, strlen(msg));
- (void)res;
+ }
+ printf("%s", msg);
Review comment:
@jojochuang This line is needed. I've replaced `write` call that was writing to **stdout** with `printf`, so that it's cross platform.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] GauthamBanasandra commented on pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#issuecomment-791875636
@smengcl could you please review my PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] GauthamBanasandra commented on pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
GauthamBanasandra commented on pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#issuecomment-787494012
Thanks for your review and suggestion @smengcl , @jojochuang. I've re-implemented my solution using the `write` system call for Windows and Linux separately, neatly tucked inside the `XPlatform` class 😁. Please do have another look at my PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jojochuang commented on a change in pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
jojochuang commented on a change in pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#discussion_r580041924
##########
File path: hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/c/connect_cancel/connect_cancel.c
##########
@@ -43,10 +42,10 @@ const char *catch_exit = "Exiting the signal handler.\n";
// Print to stdout without calling malloc or otherwise indirectly modify userspace state.
// Write calls to stdout may still interleave with stuff coming from elsewhere.
static void sighandler_direct_stdout(const char *msg) {
- if(!msg)
+ if(!msg) {
return;
- ssize_t res = write(1 /*posix stdout fd*/, msg, strlen(msg));
- (void)res;
+ }
+ printf("%s", msg);
Review comment:
was this line added to debug and not needed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] smengcl edited a comment on pull request #2710: HDFS-15843. Make write cross-platform
Posted by GitBox <gi...@apache.org>.
smengcl edited a comment on pull request #2710:
URL: https://github.com/apache/hadoop/pull/2710#issuecomment-783214670
Thanks @GauthamBanasandra for the patch.
I'm wondering if the [author](https://issues.apache.org/jira/browse/HDFS-11028) uses syscall write on purpose. According to the comment, heap could be corrupted if the `printf` call uses `malloc` or equivalent (depending on C/C++ library's implementation):
https://github.com/apache/hadoop/blob/8783461e2ec3aff2630ea3574a69beb1d5c61e84/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/c/connect_cancel/connect_cancel.c#L43
https://github.com/apache/hadoop/blob/869317be0a1fdff23be5fc500dcd9ae4ecd7bc29/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/cc/connect_cancel/connect_cancel.cc#L47-L48
How about wrapping the `<unistd.h>` header import line and the existing approach in `#ifndef WINDOWS`? And use `#else` to wrap the new approach. Assuming Windows is the platform you are aiming for.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org