[GitHub] [hadoop-ozone] fapifta commented on a change in pull request #782: HDDS-3352. Support for native ozone filesystem client using libhdfs.

[GitBox <gi...@apache.org>]

fapifta commented on a change in pull request #782: HDDS-3352. Support for native ozone filesystem client using libhdfs.
URL: https://github.com/apache/hadoop-ozone/pull/782#discussion_r405500452
 
 

 ##########
 File path: hadoop-ozone/native-client/libozone/ozfs.c
 ##########
 @@ -0,0 +1,42 @@
+#include "ozfs.h"
+#include "hdfs/hdfs.h"
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <string.h>
+
+
+ozfsFS ozfsConnect(const char *host, tPort port, const char *bucket, const char *vol)
+{
+    struct hdfsBuilder *bld = hdfsNewBuilder();
+    char string[100] = "";
+    int len = 0;
+    if (!bld)
+        return NULL;
+    len = strlen(host) + strlen(bucket) + strlen(vol) + strlen("o3fs://");
+    snprintf(string, len + 5, "o3fs://%s.%s.%s", bucket, vol, host);
 
 Review comment:
   Thank you for the contribution, I have one question here.
   
   The doc of snprintf as I understand says, it writes the output to the first param which is a char*, it writes at most the number of characters specified in the second argument, and the text is specified from the third argument on just as it should be defined for printf.
   About the char* that serves as a buffer in this case the doc says "The buffer should have a size of at least n characters." where n is the number specified in the second parameter.
   
   If these are correct, and as string is defined as a char[100], if the length of host, bucket, vol, plus the lenght of "o3fs://", plus 3 for the two dot and the null terminator is larger than 100, then as this call tries to write this many characters to the char array, it will index out from the array which will most likely cause an error.
   
   If this is true (I am not really familiar with C, but you can try this easily with a longer bucket or volume or host name), then we need to create the char array with a sufficient size to store len+5 characters.
   
   And one more thing... I am not sure why the second character of snprintf here is len+5, I see that there are two extra dots in the host, but that does not explain 5. Can you explain this a bit better in the code, with like naming it and extracting it to a local variable, or by adding a comment, that would help to understand that a bit easier.

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org