DSS-client C API update #93
Conversation
|
| keys_concat += key + "\n"; // seperate each key by line assuming there is no "\n" in the object name or directory name or delimiter | ||
| } | ||
| if (keys_concat == "") return END_OF_LIST; | ||
| keys[keys_concat.length()] = '\0'; |
There was a problem hiding this comment.
Any possibility to get length of the key during the for loop at line 1363 itself?
There was a problem hiding this comment.
yes, we can accumulate and get the sum of key lengths.
| } | ||
| if (keys_concat == "") return END_OF_LIST; | ||
| keys[keys_concat.length()] = '\0'; | ||
| strncpy(keys, keys_concat.c_str(), keys_concat.length()); |
There was a problem hiding this comment.
If above case doesn't work, you can store the keys_concat.length() in a variable and use that instead of computing length multiple times thereby introducing latency
| if (keys_concat == "") return END_OF_LIST; | ||
| keys[keys_concat.length()] = '\0'; | ||
| strncpy(keys, keys_concat.c_str(), keys_concat.length()); | ||
| if (keys[keys_concat.length()] != '\0') { |
There was a problem hiding this comment.
same as above for computing length
Is there any possibility to stop the keys_concat in the for loop itself if the memory allocation for keys is not sufficient
| return END_OF_LIST; | ||
| } | ||
| std::string keys_concat = ""; | ||
| for (std::string key: client->list_objs->GetPage()){ |
There was a problem hiding this comment.
cpp + operator is very expensive when you have lot of objects. Rather than that, you can fall back to standard and efficient C based code. But this requires key_buffer_size to be passed as a variable
len = 0
for (key: list_objs->GetPage()) {
len += snprintf(&keys[len], key_buffer_size - len, "%s", keys.c_str());
if (len ==key_buffer_size) {
//Check the last keys.length() size in the key buffer to make sure the key is completely copied.
//otherwise return error
}
}
| return FAILURE; | ||
| } | ||
| uint32_t pg_size = DSS_PAGINATION_DEFAULT; | ||
| int max_key_len = 1024; |
There was a problem hiding this comment.
Better to hash define this value (1024) as S3_MAX_KEY_SIZE_LIMIT at the top or in the header file for easy change in the future
| // calculating the size of the file | ||
| long int fsize = ftell(fp); | ||
| if (dp->d_type == DT_REG && fsize > 0){ // PutObject does not support empty files | ||
| if (PutObject(c, (void*) path, strlen(path), path) < 0 ){ |
| return -1; | ||
| } | ||
| } | ||
| if (putRecursive(c, path) < 0) return -1; |
There was a problem hiding this comment.
rather than going into the function putRecursive to check whether dir or not, you can add else case. Since you are already checking regular file in the top and you have dp->d_type handy.
| int main(int argc, char* argv[]) { | ||
| DSSClient *c; | ||
| int fd = -1; | ||
| int ret = -1; | ||
| int size = 1024*1024; | ||
| unsigned char* buff; | ||
| unsigned char* buff1; | ||
| int cur_page = -1; | ||
| int max_key_len = 1024; |
There was a problem hiding this comment.
use hash-define variable as mentioned above
| @@ -38,8 +81,9 @@ int main(int argc, char* argv[]) { | |||
| printf("Invalid endpoint URL\n"); | |||
| return -1; | |||
| } | |||
| if (argc == 5) | |||
| if (argc >= 5) | |||
There was a problem hiding this comment.
why >= is required? If so, then the usage should be updated properly
| goto out; | ||
| } | ||
| //test the LIST | ||
| keys_list = (char*) malloc(sizeof(char) * max_key_len * GetPageSize()); |
There was a problem hiding this comment.
since sizeof(char) is 1, you can see if this is really required. This may be applicable at the top as well.
|
@iso-p @grandsuri Is this a new feature? If yes - this needs to go under patent verification review. If that is the case, immediately close this PR and delete your branch. |
@velomatt @grandsuri I don't know the definition of "new feature" here. What we added was 3 extern C function to expose existing c++ code to C. |
As Mike mentioned, this is not a new feature. We are exposing the existing C++ interface functions through C code to consume in other components like .NET based. |
Added C APIs & associated tests for DSS-client: