[GitHub] [hadoop] goiri commented on a diff in pull request #4228: HDFS-16468. Define ssize_t for Windows

2022-04-26 Thread GitBox


goiri commented on code in PR #4228:
URL: https://github.com/apache/hadoop/pull/4228#discussion_r859202738


##
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/x-platform/types.h:
##
@@ -0,0 +1,39 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_TYPES
+#define NATIVE_LIBHDFSPP_LIB_CROSS_PLATFORM_TYPES
+
+#if _WIN32 || _WIN64

Review Comment:
   It might be more readable to do something like:
   ```
   #if _WIN64
   typedef long int ssize_t;
   #elif _WIN32
   typedef int ssize_t;
   #else
   #include 
   #endif
   ```
   
   With the comments and so on obviously.



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] goiri commented on a diff in pull request #4228: HDFS-16468. Define ssize_t for Windows

2022-04-26 Thread GitBox


goiri commented on code in PR #4228:
URL: https://github.com/apache/hadoop/pull/4228#discussion_r859200993


##
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/cc/cat/cat.cc:
##
@@ -62,7 +62,6 @@ int main(int argc, char *argv[]) {
   //wrapping file_raw into a unique pointer to guarantee deletion
   std::unique_ptr file(file_raw);
 
-  ssize_t total_bytes_read = 0;

Review Comment:
   I would prefer to do the cleanup of this unused variables (including the one 
in the tools) separately.



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] goiri commented on a diff in pull request #4228: HDFS-16468. Define ssize_t for Windows

2022-04-25 Thread GitBox


goiri commented on code in PR #4228:
URL: https://github.com/apache/hadoop/pull/4228#discussion_r858136011


##
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c:
##
@@ -487,10 +488,10 @@ static ssize_t wildcard_expandPath(const char* path, 
char* expanded)
  * allocated after using this function with expandedClasspath=NULL to get the
  * right size.
  */
-static ssize_t getClassPath_helper(const char *classpath, char* 
expandedClasspath)
+static x_platform_ssize_t getClassPath_helper(const char *classpath, char* 
expandedClasspath)
 {
-ssize_t length;
-ssize_t retval;
+x_platform_ssize_t length;

Review Comment:
   In some other file, we use the namespace suffix, that looks a little bit 
more maintainable than a full new type.



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[GitHub] [hadoop] goiri commented on a diff in pull request #4228: HDFS-16468. Define ssize_t for Windows

2022-04-25 Thread GitBox


goiri commented on code in PR #4228:
URL: https://github.com/apache/hadoop/pull/4228#discussion_r857865439


##
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/examples/cc/cat/cat.cc:
##
@@ -62,7 +62,6 @@ int main(int argc, char *argv[]) {
   //wrapping file_raw into a unique pointer to guarantee deletion
   std::unique_ptr file(file_raw);
 
-  ssize_t total_bytes_read = 0;

Review Comment:
   We don't use this?



##
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfs/jni_helper.c:
##
@@ -487,10 +488,10 @@ static ssize_t wildcard_expandPath(const char* path, 
char* expanded)
  * allocated after using this function with expandedClasspath=NULL to get the
  * right size.
  */
-static ssize_t getClassPath_helper(const char *classpath, char* 
expandedClasspath)
+static x_platform_ssize_t getClassPath_helper(const char *classpath, char* 
expandedClasspath)
 {
-ssize_t length;
-ssize_t retval;
+x_platform_ssize_t length;

Review Comment:
   Is this the best way to do this? Can't we just do the typedef for windows?



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org