Skip to content

Commit

Permalink
Fix cpplint and cppcheck issues
Browse files Browse the repository at this point in the history
  • Loading branch information
Silviu Caragea committed Aug 29, 2019
1 parent f3e2685 commit f709562
Show file tree
Hide file tree
Showing 17 changed files with 105 additions and 70 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
*.iml
/_build/
*.DS_Store
/cppcheck_results.xml
38 changes: 38 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,41 @@ compile_nif: get_deps

clean_nif:
@make -C c_src clean

C_SRC_DIR = $(shell pwd)/c_src
C_SRC_ENV ?= $(C_SRC_DIR)/env.mk

#regenerate all the time the env.mk
ifneq ($(wildcard $(C_SRC_DIR)),)
GEN_ENV ?= $(shell erl -noshell -s init stop -eval "file:write_file(\"$(C_SRC_ENV)\", \
io_lib:format( \
\"ERTS_INCLUDE_DIR ?= ~s/erts-~s/include/~n\" \
\"ERL_INTERFACE_INCLUDE_DIR ?= ~s~n\" \
\"ERL_INTERFACE_LIB_DIR ?= ~s~n\", \
[code:root_dir(), erlang:system_info(version), \
code:lib_dir(erl_interface, include), \
code:lib_dir(erl_interface, lib)])), \
halt().")
$(GEN_ENV)
endif

include $(C_SRC_ENV)

cpplint:
cpplint --counting=detailed \
--filter=-legal/copyright,-build/include_subdir,-build/include_order,-whitespace/blank_line,-whitespace/braces,-whitespace/indent,-whitespace/parens,-whitespace/newline \
--linelength=300 \
--exclude=c_src/*.o --exclude=c_src/*.mk \
c_src/*.*

cppcheck:
cppcheck -j 8 \
-I /usr/local/opt/openssl/include \
-I deps/librdkafka/src \
-I $(ERTS_INCLUDE_DIR) \
-I $(ERL_INTERFACE_INCLUDE_DIR) \
--force \
--enable=all \
--xml-version=2 \
--output-file=cppcheck_results.xml \
c_src/
10 changes: 5 additions & 5 deletions c_src/critical_section.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_CRITICAL_SECTION_H_
#define ERLKAF_C_SRC_CRITICAL_SECTION_H_
#ifndef C_SRC_CRITICAL_SECTION_H_
#define C_SRC_CRITICAL_SECTION_H_

#include "erl_nif.h"
#include "macros.h"
Expand All @@ -15,9 +15,9 @@ class CriticalSection
void Leave() {enif_mutex_unlock(mutex_);}

private:
ErlNifMutex *mutex_;

DISALLOW_COPY_AND_ASSIGN(CriticalSection);
ErlNifMutex *mutex_;
};

class CritScope
Expand All @@ -28,9 +28,9 @@ class CritScope
~CritScope() {pcrit_->Leave();}

private:
CriticalSection *pcrit_;

DISALLOW_COPY_AND_ASSIGN(CritScope);
CriticalSection *pcrit_;
};

#endif
#endif // C_SRC_CRITICAL_SECTION_H_
5 changes: 3 additions & 2 deletions c_src/erlkaf_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <functional>
#include <signal.h>
#include <string>

namespace {

Expand Down Expand Up @@ -65,7 +66,7 @@ bool appy_kafka_default_config(rd_kafka_conf_t* config)
return false;

#ifdef SIGIO
//quick termination
// quick termination
char tmp[128];
snprintf(tmp, sizeof(tmp), "%i", SIGIO);

Expand All @@ -82,7 +83,7 @@ bool appy_topic_default_config(rd_kafka_topic_conf_t* config)
return true;
}

}
} // namespace

ERL_NIF_TERM parse_topic_config(ErlNifEnv* env, ERL_NIF_TERM list, rd_kafka_topic_conf_t* conf)
{
Expand Down
6 changes: 3 additions & 3 deletions c_src/erlkaf_config.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_ERLKAF_CONFIG_H_
#define ERLKAF_C_SRC_ERLKAF_CONFIG_H_
#ifndef C_SRC_ERLKAF_CONFIG_H_
#define C_SRC_ERLKAF_CONFIG_H_

#include "erl_nif.h"

Expand All @@ -9,5 +9,5 @@ typedef struct rd_kafka_topic_conf_s rd_kafka_topic_conf_t;
ERL_NIF_TERM parse_topic_config(ErlNifEnv* env, ERL_NIF_TERM list, rd_kafka_topic_conf_t* conf);
ERL_NIF_TERM parse_kafka_config(ErlNifEnv* env, ERL_NIF_TERM list, rd_kafka_conf_t* conf);

#endif
#endif // C_SRC_ERLKAF_CONFIG_H_

25 changes: 13 additions & 12 deletions c_src/erlkaf_consumer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "queuemanager.h"

#include <vector>
#include <string>
#include <memory>
#include <string.h>
#include <unistd.h>
Expand Down Expand Up @@ -93,16 +94,16 @@ void* consumer_poll_thread(void* arg)

if(msg)
{
//because communication between nif and erlang it's based on async messages might be a small window
//between starting of revoking partitions (queued are forwarded back on the main queue) and when actual we revoked them
//when we get the messages here. we drop all this messages as time they have no impact because offset is not changed.
//we are sleeping here as well to not consume lot of cpu
// because communication between nif and erlang it's based on async messages might be a small window
// between starting of revoking partitions (queued are forwarded back on the main queue) and when actual we revoked them
// when we get the messages here. we drop all this messages as time they have no impact because offset is not changed.
// we are sleeping here as well to not consume lot of cpu
rd_kafka_message_destroy(msg);
usleep(50000);
}
}

//make sure queues are forwarded back on the main queue before closing
// make sure queues are forwarded back on the main queue before closing
consumer->qm_->clear_all();

rd_kafka_consumer_close(consumer->kf);
Expand Down Expand Up @@ -223,7 +224,7 @@ ERL_NIF_TERM get_headers(ErlNifEnv* env, const rd_kafka_message_t* msg)
return ATOMS.atomUndefined;
}

}
} // namespace

void enif_queue_free(ErlNifEnv* env, void* obj)
{
Expand Down Expand Up @@ -252,7 +253,7 @@ void enif_consumer_free(ErlNifEnv* env, void* obj)
}

if(consumer->qm_)
delete consumer->qm_;
delete consumer->qm_;
}

ERL_NIF_TERM enif_consumer_new(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
Expand Down Expand Up @@ -342,7 +343,7 @@ ERL_NIF_TERM enif_consumer_queue_cleanup(ErlNifEnv* env, int argc, const ERL_NIF

enif_queue* q;

if(!enif_get_resource(env, argv[0], data->res_queue, (void**) &q))
if(!enif_get_resource(env, argv[0], data->res_queue, reinterpret_cast<void**>(&q)))
return make_badarg(env);

q->consumer->qm_->remove(q->queue);
Expand All @@ -358,7 +359,7 @@ ERL_NIF_TERM enif_consumer_partition_revoke_completed(ErlNifEnv* env, int argc,
erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));
enif_consumer* c;

if(!enif_get_resource(env, argv[0], data->res_consumer, (void**) &c))
if(!enif_get_resource(env, argv[0], data->res_consumer, reinterpret_cast<void**>(&c)))
return make_badarg(env);

rd_kafka_assign(c->kf, NULL);
Expand All @@ -372,7 +373,7 @@ ERL_NIF_TERM enif_consumer_queue_poll(ErlNifEnv* env, int argc, const ERL_NIF_TE
erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));
enif_queue* q;

if(!enif_get_resource(env, argv[0], data->res_queue, (void**) &q))
if(!enif_get_resource(env, argv[0], data->res_queue, reinterpret_cast<void**>(&q)))
return make_badarg(env);

uint32_t max_batch_size;
Expand Down Expand Up @@ -439,7 +440,7 @@ ERL_NIF_TERM enif_consumer_offset_store(ErlNifEnv* env, int argc, const ERL_NIF_
erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));
enif_consumer* c;

if(!enif_get_resource(env, argv[0], data->res_consumer, (void**) &c))
if(!enif_get_resource(env, argv[0], data->res_consumer, reinterpret_cast<void**>(&c)))
return make_badarg(env);

if(!get_string(env, argv[1], &topic_name))
Expand Down Expand Up @@ -468,7 +469,7 @@ ERL_NIF_TERM enif_consumer_cleanup(ErlNifEnv* env, int argc, const ERL_NIF_TERM
erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));
enif_consumer* consumer;

if(!enif_get_resource(env, argv[0], data->res_consumer, (void**) &consumer))
if(!enif_get_resource(env, argv[0], data->res_consumer, reinterpret_cast<void**>(&consumer)))
return make_badarg(env);

consumer->stop_feedback = true;
Expand Down
6 changes: 3 additions & 3 deletions c_src/erlkaf_consumer.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_ERLKAF_CONSUMER_H_
#define ERLKAF_C_SRC_ERLKAF_CONSUMER_H_
#ifndef C_SRC_ERLKAF_CONSUMER_H_
#define C_SRC_ERLKAF_CONSUMER_H_

#include "erl_nif.h"

Expand All @@ -12,4 +12,4 @@ ERL_NIF_TERM enif_consumer_queue_cleanup(ErlNifEnv* env, int argc, const ERL_NIF
ERL_NIF_TERM enif_consumer_offset_store(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]);
ERL_NIF_TERM enif_consumer_cleanup(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]);

#endif
#endif // C_SRC_ERLKAF_CONSUMER_H_
6 changes: 3 additions & 3 deletions c_src/erlkaf_logger.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_ERLKAF_LOGGER_H_
#define ERLKAF_C_SRC_ERLKAF_LOGGER_H_
#ifndef C_SRC_ERLKAF_LOGGER_H_
#define C_SRC_ERLKAF_LOGGER_H_

#include "erlkaf_nif.h"

Expand All @@ -23,4 +23,4 @@ ERL_NIF_TERM nif_set_logger_pid(ErlNifEnv* env, int argc, const ERL_NIF_TERM arg

void log_message(const rd_kafka_t *rk, kRdLogLevel level, const std::string& msg);

#endif
#endif // C_SRC_ERLKAF_LOGGER_H_
6 changes: 3 additions & 3 deletions c_src/erlkaf_nif.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_ERLKAF_NIF_H_
#define ERLKAF_C_SRC_ERLKAF_NIF_H_
#ifndef C_SRC_ERLKAF_NIF_H_
#define C_SRC_ERLKAF_NIF_H_

#include "erl_nif.h"

Expand Down Expand Up @@ -30,4 +30,4 @@ struct erlkaf_data

extern atoms ATOMS;

#endif
#endif // C_SRC_ERLKAF_NIF_H_
11 changes: 6 additions & 5 deletions c_src/erlkaf_producer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <string.h>
#include <memory>
#include <string>

namespace {

Expand Down Expand Up @@ -106,7 +107,7 @@ bool populate_headers(ErlNifEnv* env, ERL_NIF_TERM headers_term, rd_kafka_header
return true;
}

}
} // namespace

void enif_producer_free(ErlNifEnv* env, void* obj)
{
Expand Down Expand Up @@ -138,7 +139,7 @@ ERL_NIF_TERM enif_producer_topic_new(ErlNifEnv* env, int argc, const ERL_NIF_TER

erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));

if(!enif_get_resource(env, argv[0], data->res_producer, (void**) &producer))
if(!enif_get_resource(env, argv[0], data->res_producer, reinterpret_cast<void**>(&producer)))
return make_badarg(env);

if(!get_string(env, argv[1], &topic_name))
Expand Down Expand Up @@ -224,7 +225,7 @@ ERL_NIF_TERM enif_producer_set_owner(ErlNifEnv* env, int argc, const ERL_NIF_TER

enif_producer* producer;

if(!enif_get_resource(env, argv[0], data->res_producer, (void**) &producer))
if(!enif_get_resource(env, argv[0], data->res_producer, reinterpret_cast<void**>(&producer)))
return make_badarg(env);

if(!enif_get_local_pid(env, argv[1], &producer->owner_pid))
Expand All @@ -240,7 +241,7 @@ ERL_NIF_TERM enif_producer_cleanup(ErlNifEnv* env, int argc, const ERL_NIF_TERM
erlkaf_data* data = static_cast<erlkaf_data*>(enif_priv_data(env));
enif_producer* producer;

if(!enif_get_resource(env, argv[0], data->res_producer, (void**) &producer))
if(!enif_get_resource(env, argv[0], data->res_producer, reinterpret_cast<void**>(&producer)))
return make_badarg(env);

producer->stop_feedback = true;
Expand All @@ -263,7 +264,7 @@ ERL_NIF_TERM enif_produce(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])

scoped_ptr(headers, rd_kafka_headers_t, NULL, rd_kafka_headers_destroy);

if(!enif_get_resource(env, argv[0], data->res_producer, (void**) &producer))
if(!enif_get_resource(env, argv[0], data->res_producer, reinterpret_cast<void**>(&producer)))
return make_badarg(env);

if(!get_string(env, argv[1], &topic_name))
Expand Down
6 changes: 3 additions & 3 deletions c_src/erlkaf_producer.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_ERLKAF_PRODUCER_H_
#define ERLKAF_C_SRC_ERLKAF_PRODUCER_H_
#ifndef C_SRC_ERLKAF_PRODUCER_H_
#define C_SRC_ERLKAF_PRODUCER_H_

#include "erl_nif.h"

Expand All @@ -11,4 +11,4 @@ ERL_NIF_TERM enif_producer_set_owner(ErlNifEnv* env, int argc, const ERL_NIF_TER
ERL_NIF_TERM enif_producer_cleanup(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]);
ERL_NIF_TERM enif_produce(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]);

#endif
#endif // C_SRC_ERLKAF_PRODUCER_H_
6 changes: 3 additions & 3 deletions c_src/macros.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_MACROS_H_
#define ERLKAF_C_SRC_MACROS_H_
#ifndef C_SRC_MACROS_H_
#define C_SRC_MACROS_H_

#define UNUSED(expr) do { (void)(expr); } while (0)

Expand All @@ -16,4 +16,4 @@
#define ASSERT(x) assert(x)
#endif

#endif
#endif // C_SRC_MACROS_H_
6 changes: 3 additions & 3 deletions c_src/nif_utils.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef ERLKAF_C_SRC_NIF_UTILS_H_
#define ERLKAF_C_SRC_NIF_UTILS_H_
#ifndef C_SRC_NIF_UTILS_H_
#define C_SRC_NIF_UTILS_H_

#include <stdint.h>
#include <string>
Expand All @@ -18,4 +18,4 @@ bool get_binary(ErlNifEnv* env, ERL_NIF_TERM term, ErlNifBinary* bin);
bool get_string(ErlNifEnv *env, ERL_NIF_TERM term, std::string* var);
bool get_boolean(ERL_NIF_TERM term, bool* val);

#endif
#endif // C_SRC_NIF_UTILS_H_
13 changes: 5 additions & 8 deletions c_src/queuemanager.cc
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
#include "queuemanager.h"
#include "rdkafka.h"

QueueManager::QueueManager(rd_kafka_t *rk) : rk_(rk)
{

}
QueueManager::QueueManager(rd_kafka_t *rk) : rk_(rk) { }

QueueManager::~QueueManager()
{
Expand All @@ -16,7 +13,7 @@ void QueueManager::add(rd_kafka_queue_t* queue)
CritScope ss(&crt_);
ASSERT(queues_.find(queue) == queues_.end());

//remove the queue forwarding on the main queue.
// remove the queue forwarding on the main queue.
rd_kafka_queue_forward(queue, NULL);
queues_.insert(queue);
}
Expand All @@ -29,7 +26,7 @@ bool QueueManager::remove(rd_kafka_queue_t* queue)
if(it == queues_.end())
return false;

//forward the queue back to the main queue
// forward the queue back to the main queue
rd_kafka_queue_t* main_queue = rd_kafka_queue_get_consumer(rk_);
rd_kafka_queue_forward(*it, main_queue);
rd_kafka_queue_destroy(main_queue);
Expand All @@ -42,9 +39,9 @@ void QueueManager::clear_all()
{
CritScope ss(&crt_);

//forwards all queues back on the main queue
// forwards all queues back on the main queue

for(auto it = queues_.begin(); it != queues_.end(); ++ it)
for(auto it = queues_.begin(); it != queues_.end(); ++it)
{
rd_kafka_queue_t* main_queue = rd_kafka_queue_get_consumer(rk_);
rd_kafka_queue_forward(*it, main_queue);
Expand Down
Loading

0 comments on commit f709562

Please sign in to comment.