Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings #10

Merged
merged 11 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ target_link_libraries(khiopsdriver_file_s3 PRIVATE ${AWSSDK_LIBRARIES}

target_compile_options(
khiopsdriver_file_s3
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:-Wall>
PRIVATE
$<$<CXX_COMPILER_ID:MSVC>:-Wall;/wd4582;/wd4583;/wd4625;/wd4626;/wd4710;/wd4711;/wd4820;/wd4868>
PRIVATE $<$<CXX_COMPILER_ID:AppleClang,Clang,GNU>:-Wall;-Wextra;-pedantic>)

option(BUILD_TESTS "Build test programs" OFF)
Expand Down
13 changes: 11 additions & 2 deletions src/ini.h → src/contrib/ini.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@
#include <utility>
#include <vector>

#ifdef _WIN32
#pragma warning( push )
#pragma warning(disable:4514)
#endif

namespace mINI {
namespace INIStringUtil {
const char *const whitespaceDelimiters = " \t\n\r\f\v";
Expand Down Expand Up @@ -322,7 +327,7 @@ class INIReader {
std::string fileContents;
fileContents.resize(fileSize);
fileReadStream.seekg(isBOM ? 3 : 0, std::ios::beg);
fileReadStream.read(&fileContents[0], fileSize);
fileReadStream.read(&fileContents[0], static_cast<std::streamsize>(fileSize));
fileReadStream.close();
T_LineData output;
if (fileSize == 0) {
Expand Down Expand Up @@ -530,7 +535,7 @@ class INIWriter {
}
}
if (!linesToAdd.empty()) {
output.insert(output.begin() + lastKeyLine, linesToAdd.begin(),
output.insert(output.begin() + static_cast<int64_t>(lastKeyLine), linesToAdd.begin(),
linesToAdd.end());
}
if (writeNewKeys) {
Expand Down Expand Up @@ -651,4 +656,8 @@ class INIFile {
};
} // namespace mINI

#ifdef _WIN32
#pragma warning( pop )
#endif

#endif // MINI_INI_H_
53 changes: 26 additions & 27 deletions src/s3plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,16 +211,6 @@ template <typename H> void EraseRemove(HandleContainer<H>& container, HandleIt<H
container.pop_back();
}

struct SimpleErrorModel
{
Aws::String err_msg_;

const Aws::String& GetMessage() const
{
return err_msg_;
}
};

struct SimpleError
{
int code_;
Expand All @@ -230,11 +220,6 @@ struct SimpleError
{
return std::to_string(code_) + err_msg_;
}

SimpleErrorModel GetModeledError() const
{
return {GetMessage()};
}
};

SimpleError MakeSimpleError(Aws::S3::S3Errors err_code, Aws::String&& err_msg)
Expand Down Expand Up @@ -289,7 +274,7 @@ SizeOutcome DownloadFileRangeToVector(const Aws::String& bucket, const Aws::Stri

// Convert string to vector<char>
contentVector.assign(objectData.begin(), objectData.end());
return objectData.size();
return static_cast<long long>(objectData.size());
}

SizeOutcome DownloadFileRangeToBuffer(const Aws::String& bucket, const Aws::String& object_name, unsigned char* buffer,
Expand Down Expand Up @@ -473,8 +458,19 @@ ParseURIOutcome ParseS3Uri(const Aws::String& s3_uri)

Aws::String GetEnvironmentVariableOrDefault(const Aws::String& variable_name, const Aws::String& default_value)
{
const char* value = std::getenv(variable_name.c_str());
return value ? value : default_value;
#ifdef _WIN32
size_t len;
char value[2048];
getenv_s(&len, value, 2048, variable_name.c_str());
#else
char *value = getenv(variable_name.c_str());
#endif

if (value && std::strlen(value) > 0) {
return value;
} else {
return default_value;
}
}

bool IsMultifile(const Aws::String& pattern, size_t& first_special_char_idx)
Expand Down Expand Up @@ -765,8 +761,11 @@ int driver_connect()
Aws::InitAPI(options);

Aws::Client::ClientConfiguration clientConfig(true, "legacy", true);
clientConfig.allowSystemProxy = getenv("http_proxy") != NULL || getenv("https_proxy") != NULL ||
getenv("HTTP_PROXY") != NULL || getenv("HTTPS_PROXY") != NULL || getenv("S3_ALLOW_SYSTEM_PROXY");
clientConfig.allowSystemProxy = !GetEnvironmentVariableOrDefault("http_proxy", "").empty() ||
!GetEnvironmentVariableOrDefault("https_proxy", "").empty() ||
!GetEnvironmentVariableOrDefault("HTTP_PROXY", "").empty() ||
!GetEnvironmentVariableOrDefault("HTTPS_PROXY", "").empty() ||
!GetEnvironmentVariableOrDefault("S3_ALLOW_SYSTEM_PROXY", "").empty();
clientConfig.verifySSL = true;
clientConfig.version = Aws::Http::Version::HTTP_VERSION_2TLS;
if (s3endpoint != "")
Expand Down Expand Up @@ -1020,7 +1019,7 @@ SizeOutcome getFileSize(const Aws::String& bucket_name, const Aws::String& objec
{
nb_headers_to_subtract = 0;
}
return total_size - nb_headers_to_subtract * header_size;
return total_size - static_cast<long long>(nb_headers_to_subtract * header_size);
}

long long int driver_getFileSize(const char* filename)
Expand Down Expand Up @@ -1214,8 +1213,8 @@ UploadOutcome InitiateAppend(Writer& writer, size_t source_bytes_to_copy)
int64_t start_range = 0;
while (source_bytes_to_copy > Writer::buff_min_)
{
const size_t copy_count =
source_bytes_to_copy > Writer::buff_max_ ? Writer::buff_max_ : source_bytes_to_copy;
const int64_t copy_count =
static_cast<int64_t>(source_bytes_to_copy > Writer::buff_max_ ? Writer::buff_max_ : source_bytes_to_copy);

// peculiarity of AWS: the range for the copy request has an inclusive end,
// meaning that the bytes numbered start_range to end_range included are copied
Expand All @@ -1234,7 +1233,7 @@ UploadOutcome InitiateAppend(Writer& writer, size_t source_bytes_to_copy)
// reminder: byte ranges are inclusive
auto outcome = DownloadFileRangeToVector(multipartupload_data.GetBucket(),
multipartupload_data.GetKey(), writer.buffer_,
start_range, start_range + source_bytes_to_copy - 1);
start_range, start_range + static_cast<int64_t>(source_bytes_to_copy) - 1);
PASS_OUTCOME_ON_ERROR(outcome);

tOffset actual_read = outcome.GetResult();
Expand Down Expand Up @@ -1330,7 +1329,7 @@ void* driver_fopen(const char* filename, char mode)
return writer_ptr;
}
default:
LogError("Invalid open mode " + mode);
LogError(std::string("Invalid open mode: ") + mode);
return nullptr;
}
}
Expand Down Expand Up @@ -1599,7 +1598,7 @@ long long int driver_fwrite(const void* ptr, size_t size, size_t count, void* st
// release unused memory
buffer.shrink_to_fit();

return to_write;
return static_cast<long long>(to_write);
}

int driver_fflush(void*)
Expand Down Expand Up @@ -1839,5 +1838,5 @@ bool test_compareFiles(const char* local_file_path_str, const char* s3_uri_str)
// Comparer les contenus
auto result = local_content == s3_content.str() ? kSuccess : kFailure;

return result;
return static_cast<bool>(result);
}
6 changes: 3 additions & 3 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ add_executable(basic_test basic_test.cpp drivertest.cpp)

target_compile_options(
basic_test
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:-Wall>
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/W4;/wd4101;/wd4625;/wd4710;/wd4711>
PRIVATE $<$<CXX_COMPILER_ID:AppleClang,Clang,GNU>:-Wall;-Wextra;-pedantic>)

# MSVC requires /bigobj to build in debug config
Expand All @@ -40,14 +40,14 @@ target_link_libraries(basic_test PRIVATE GTest::gtest GTest::gmock
GTest::gmock_main khiopsdriver_file_s3)

if (WIN32)
target_link_libraries(basic_test PRIVATE ws2_32 Wininet winhttp)
target_link_libraries(basic_test PRIVATE ws2_32)
endif()
gtest_discover_tests(basic_test)

add_executable(plugin_test plugin_test.cpp path_helper.cpp)
target_compile_options(
plugin_test
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:-Wall>
PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/W4;/wd4101;/wd4710;/wd4711>
PRIVATE $<$<CXX_COMPILER_ID:AppleClang,Clang,GNU>:-Wall;-Wextra;-pedantic>)
if(WIN32)
# custom build and test execution for Windows
Expand Down
13 changes: 8 additions & 5 deletions test/drivertest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,11 @@ int launch_test(const char *inputFilename, int nBufferSize) {
<< boost::uuids::random_generator()() << "/output.txt";
std::stringstream localOutput;
#ifdef _WIN32
localOutput << std::getenv("TEMP") << "\\out-"
<< boost::uuids::random_generator()() << ".txt";
size_t len;
char tempValue[2048];
getenv_s(&len, tempValue, 2048, "TEMP");
localOutput << tempValue << "\\out-" << boost::uuids::random_generator()()
<< ".txt";
#else
localOutput << "/tmp/out-" << boost::uuids::random_generator()() << ".txt";
#endif
Expand Down Expand Up @@ -334,9 +337,9 @@ int copyFileWithFseek(const char *file_name_input, const char *file_name_output,
// Reads the file by steps of nBufferSize and writes to the output file at
// each step
char *buffer = new char[nBufferSize + 1]();
long long int sizeRead = nBufferSize;
long long int sizeWrite;
int cummulativeRead = 0;
long long sizeRead = nBufferSize;
long long sizeWrite;
long long cummulativeRead = 0;
driver_fseek(fileinput, 0, SEEK_SET);
while (sizeRead == nBufferSize && copy_status == kSuccess) {
driver_fseek(fileinput, cummulativeRead, SEEK_SET);
Expand Down
5 changes: 3 additions & 2 deletions test/path_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ std::string getExecutablePath() {

std::string getExecutableDir() {
std::string executablePath = getExecutablePath();
char *exePath = new char[executablePath.length()];
strcpy(exePath, executablePath.c_str());
char *exePath = new char[executablePath.length() + 1];
strncpy_s(exePath, executablePath.length() + 1, executablePath.c_str(),
executablePath.length() + 1);
PathRemoveFileSpec(exePath);
std::string directory = std::string(exePath);
delete[] exePath;
Expand Down