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

Add clang-format #291

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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: 3 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Language: Cpp

BasedOnStyle: Google
1 change: 1 addition & 0 deletions .gitpod.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ RUN sudo apt install -y \
r-base \
ccache \
cmake \
clang-format \
mariadb-server mariadb-client \
# Install dependencies for devtools package
libharfbuzz-dev libfribidi-dev
6 changes: 4 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
cmake_minimum_required(VERSION 3.14)

project(RMariaDB VERSION 0.1.0)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/")
include(Misc)

add_subdirectory(src)
add_subdirectory(src)
24 changes: 24 additions & 0 deletions cmake/Misc.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

# clang-format support
function(add_clang_format_target)
set(options)
set(oneValueArgs)
set(multiValueArgs SOURCES)
cmake_parse_arguments(ARG "${options}" "${oneValueArgs}"
"${multiValueArgs}" ${ARGN})
if(NOT ${PROJECT_NAME}_CLANG_FORMAT_BINARY)
find_program(${PROJECT_NAME}_CLANG_FORMAT_BINARY clang-format)
endif()

message(STATUS ${ARG_SOURCES})

if(${PROJECT_NAME}_CLANG_FORMAT_BINARY)
add_custom_target(clang-format
COMMAND ${${PROJECT_NAME}_CLANG_FORMAT_BINARY} --verbose
-i ${ARG_SOURCES}
WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR})

message(STATUS "Format the project using the `clang-format` target (i.e: cmake --build build --target clang-format).\n")
endif()
endfunction()
59 changes: 31 additions & 28 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,39 @@ get_filename_component(MYSQL_PATH ${MYSQL} DIRECTORY)
find_file(POSTGRESSQL postgresql/plogr.h)
get_filename_component(POSTGRESSQL_PATH ${POSTGRESSQL} DIRECTORY)

add_library(RMariaDB
connection.cpp
DbConnection.cpp
DbConnection.h
DbResult.cpp
DbResult.h
DbResultImplDecl.h
DbResultImpl.h
driver.cpp
integer64.h
MariaBinding.cpp
MariaBinding.h
MariaResult.cpp
MariaResult.h
MariaResultImpl.cpp
MariaResultImpl.h
MariaResultPrep.cpp
MariaResultPrep.h
MariaResultSimple.cpp
MariaResultSimple.h
MariaRow.cpp
MariaRow.h
MariaTypes.cpp
MariaTypes.h
pch.h
RcppExports.cpp
result.cpp
RMariaDB_types.h
set(SOURCES
connection.cpp
DbConnection.cpp
DbConnection.h
DbResult.cpp
DbResult.h
DbResultImplDecl.h
DbResultImpl.h
driver.cpp
integer64.h
MariaBinding.cpp
MariaBinding.h
MariaResult.cpp
MariaResult.h
MariaResultImpl.cpp
MariaResultImpl.h
MariaResultPrep.cpp
MariaResultPrep.h
MariaResultSimple.cpp
MariaResultSimple.h
MariaRow.cpp
MariaRow.h
MariaTypes.cpp
MariaTypes.h
pch.h
RcppExports.cpp
result.cpp
RMariaDB_types.h
)

add_library(RMariaDB ${SOURCES})
add_clang_format_target(SOURCES ${SOURCES})

target_include_directories(RMariaDB PUBLIC
"/usr/share/R/include"
"/home/gitpod/R/x86_64-pc-linux-gnu-library/3.6/Rcpp/include/"
Expand Down
114 changes: 47 additions & 67 deletions src/DbConnection.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
#include "pch.h"
#include "DbConnection.h"

#include "DbResult.h"
#include "pch.h"

DbConnection::DbConnection() :
pConn_(NULL),
pCurrentResult_(NULL),
transacting_(false)
{
DbConnection::DbConnection()
: pConn_(NULL), pCurrentResult_(NULL), transacting_(false) {
LOG_VERBOSE;
}

Expand All @@ -19,15 +17,16 @@ DbConnection::~DbConnection() {
}
}

void DbConnection::connect(const Nullable<std::string>& host, const Nullable<std::string>& user,
const Nullable<std::string>& password, const Nullable<std::string>& db,
unsigned int port, const Nullable<std::string>& unix_socket,
unsigned long client_flag, const Nullable<std::string>& groups,
const Nullable<std::string>& default_file,
const Nullable<std::string>& ssl_key, const Nullable<std::string>& ssl_cert,
const Nullable<std::string>& ssl_ca, const Nullable<std::string>& ssl_capath,
const Nullable<std::string>& ssl_cipher,
int timeout, bool reconnect) {
void DbConnection::connect(
const Nullable<std::string>& host, const Nullable<std::string>& user,
const Nullable<std::string>& password, const Nullable<std::string>& db,
unsigned int port, const Nullable<std::string>& unix_socket,
unsigned long client_flag, const Nullable<std::string>& groups,
const Nullable<std::string>& default_file,
const Nullable<std::string>& ssl_key, const Nullable<std::string>& ssl_cert,
const Nullable<std::string>& ssl_ca,
const Nullable<std::string>& ssl_capath,
const Nullable<std::string>& ssl_cipher, int timeout, bool reconnect) {
LOG_VERBOSE;

this->pConn_ = mysql_init(NULL);
Expand All @@ -46,33 +45,30 @@ void DbConnection::connect(const Nullable<std::string>& host, const Nullable<std
if (!ssl_key.isNull() || !ssl_cert.isNull() || !ssl_ca.isNull() ||
!ssl_capath.isNull() || !ssl_cipher.isNull()) {
mysql_ssl_set(
this->pConn_,
ssl_key.isNull() ? NULL : as<std::string>(ssl_key).c_str(),
ssl_cert.isNull() ? NULL : as<std::string>(ssl_cert).c_str(),
ssl_ca.isNull() ? NULL : as<std::string>(ssl_ca).c_str(),
ssl_capath.isNull() ? NULL : as<std::string>(ssl_capath).c_str(),
ssl_cipher.isNull() ? NULL : as<std::string>(ssl_cipher).c_str()
);
this->pConn_,
ssl_key.isNull() ? NULL : as<std::string>(ssl_key).c_str(),
ssl_cert.isNull() ? NULL : as<std::string>(ssl_cert).c_str(),
ssl_ca.isNull() ? NULL : as<std::string>(ssl_ca).c_str(),
ssl_capath.isNull() ? NULL : as<std::string>(ssl_capath).c_str(),
ssl_cipher.isNull() ? NULL : as<std::string>(ssl_cipher).c_str());
}
if (timeout > 0) {
mysql_options(this->pConn_, MYSQL_OPT_CONNECT_TIMEOUT,
&timeout);
mysql_options(this->pConn_, MYSQL_OPT_CONNECT_TIMEOUT, &timeout);
}
if (reconnect) {
my_bool reconnect_ = 1;
mysql_options(this->pConn_, MYSQL_OPT_RECONNECT, (void *)&reconnect_);
mysql_options(this->pConn_, MYSQL_OPT_RECONNECT, (void*)&reconnect_);
}

LOG_VERBOSE;

if (!mysql_real_connect(this->pConn_,
host.isNull() ? NULL : as<std::string>(host).c_str(),
user.isNull() ? NULL : as<std::string>(user).c_str(),
password.isNull() ? NULL : as<std::string>(password).c_str(),
db.isNull() ? NULL : as<std::string>(db).c_str(),
port,
unix_socket.isNull() ? NULL : as<std::string>(unix_socket).c_str(),
client_flag)) {
if (!mysql_real_connect(
this->pConn_, host.isNull() ? NULL : as<std::string>(host).c_str(),
user.isNull() ? NULL : as<std::string>(user).c_str(),
password.isNull() ? NULL : as<std::string>(password).c_str(),
db.isNull() ? NULL : as<std::string>(db).c_str(), port,
unix_socket.isNull() ? NULL : as<std::string>(unix_socket).c_str(),
client_flag)) {
std::string error = mysql_error(this->pConn_);
mysql_close(this->pConn_);
this->pConn_ = NULL;
Expand All @@ -85,23 +81,19 @@ void DbConnection::disconnect() {
if (!is_valid()) return;

if (has_query()) {
warning(
"%s\n%s",
"There is a result object still in use.",
"The connection will be automatically released when it is closed"
);
warning("%s\n%s", "There is a result object still in use.",
"The connection will be automatically released when it is closed");
}

try {
mysql_close(get_conn());
} catch (...) {};
} catch (...) {
};

pConn_ = NULL;
}

bool DbConnection::is_valid() {
return !!get_conn();
}
bool DbConnection::is_valid() { return !!get_conn(); }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather avoid braces on a single line.


void DbConnection::check_connection() {
if (!is_valid()) {
Expand All @@ -110,26 +102,21 @@ void DbConnection::check_connection() {
}

List DbConnection::info() {
return
List::create(
return List::create(
_["host"] = std::string(pConn_->host),
_["username"] = std::string(pConn_->user),
_["dbname"] = std::string(pConn_->db ? pConn_->db : ""),
_["con.type"] = std::string(mysql_get_host_info(pConn_)),
_["db.version"] = std::string(mysql_get_server_info(pConn_)),
_["port"] = NA_INTEGER,
_["protocol.version"] = (int) mysql_get_proto_info(pConn_),
_["thread.id"] = (int) mysql_thread_id(pConn_)
);
_["protocol.version"] = (int)mysql_get_proto_info(pConn_),
_["thread.id"] = (int)mysql_thread_id(pConn_));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the closing paren ); to the next line?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature will be added to clang-format. Seems now doesn't exists.

}

MYSQL* DbConnection::get_conn() {
return pConn_;
}
MYSQL* DbConnection::get_conn() { return pConn_; }

SEXP DbConnection::quote_string(const String& input) {
if (input == NA_STRING)
return get_null_string();
if (input == NA_STRING) return get_null_string();

const char* input_cstr = input.get_cstring();
size_t input_len = strlen(input_cstr);
Expand All @@ -138,7 +125,8 @@ SEXP DbConnection::quote_string(const String& input) {
std::string output = "'";
output.resize(input_len * 2 + 3);

size_t end = mysql_real_escape_string(pConn_, &output[1], input_cstr, input_len);
size_t end =
mysql_real_escape_string(pConn_, &output[1], input_cstr, input_len);

output.resize(end + 1);
output.append("'");
Expand All @@ -151,12 +139,10 @@ SEXP DbConnection::get_null_string() {
}

void DbConnection::set_current_result(DbResult* pResult) {
if (pResult == pCurrentResult_)
return;
if (pResult == pCurrentResult_) return;

if (pCurrentResult_ != NULL) {
if (pResult != NULL)
warning("Cancelling previous query");
if (pResult != NULL) warning("Cancelling previous query");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to always add braces, but keep it on a new line.


pCurrentResult_->close();
}
Expand All @@ -165,8 +151,7 @@ void DbConnection::set_current_result(DbResult* pResult) {

void DbConnection::reset_current_result(DbResult* pResult) {
// FIXME: What to do if not current result is reset?
if (pResult != pCurrentResult_)
return;
if (pResult != pCurrentResult_) return;

pCurrentResult_->close();
pCurrentResult_ = NULL;
Expand All @@ -176,9 +161,7 @@ bool DbConnection::is_current_result(const DbResult* pResult) const {
return pCurrentResult_ == pResult;
}

bool DbConnection::has_query() {
return pCurrentResult_ != NULL;
}
bool DbConnection::has_query() { return pCurrentResult_ != NULL; }

bool DbConnection::exec(const std::string& sql) {
check_connection();
Expand All @@ -187,8 +170,7 @@ bool DbConnection::exec(const std::string& sql) {
stop("Error executing query: %s", mysql_error(pConn_));

MYSQL_RES* res = mysql_store_result(pConn_);
if (res != NULL)
mysql_free_result(res);
if (res != NULL) mysql_free_result(res);

autocommit();

Expand Down Expand Up @@ -218,9 +200,7 @@ void DbConnection::rollback() {
transacting_ = false;
}

bool DbConnection::is_transacting() const {
return transacting_;
}
bool DbConnection::is_transacting() const { return transacting_; }

void DbConnection::autocommit() {
if (!is_transacting() && get_conn()) {
Expand Down
26 changes: 15 additions & 11 deletions src/DbConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,24 @@ class DbConnection : boost::noncopyable {
DbResult* pCurrentResult_;
bool transacting_;

public:

public:
DbConnection();
~DbConnection();

public:
void
connect(const Nullable<std::string>& host, const Nullable<std::string>& user, const Nullable<std::string>& password,
const Nullable<std::string>& db, unsigned int port, const Nullable<std::string>& unix_socket,
unsigned long client_flag, const Nullable<std::string>& groups, const Nullable<std::string>& default_file,
const Nullable<std::string>& ssl_key, const Nullable<std::string>& ssl_cert,
const Nullable<std::string>& ssl_ca, const Nullable<std::string>& ssl_capath,
const Nullable<std::string>& ssl_cipher,
int timeout, bool reconnect);
public:
void connect(const Nullable<std::string>& host,
const Nullable<std::string>& user,
const Nullable<std::string>& password,
const Nullable<std::string>& db, unsigned int port,
const Nullable<std::string>& unix_socket,
unsigned long client_flag, const Nullable<std::string>& groups,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we ensure that each argument gets its own line in these cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think now it's not possible. Clang-format add new line if arguments are to long, but it doesn't check the context of this lines.

const Nullable<std::string>& default_file,
const Nullable<std::string>& ssl_key,
const Nullable<std::string>& ssl_cert,
const Nullable<std::string>& ssl_ca,
const Nullable<std::string>& ssl_capath,
const Nullable<std::string>& ssl_cipher, int timeout,
bool reconnect);
void disconnect();
bool is_valid();
void check_connection();
Expand Down
Loading