You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by li...@apache.org on 2023/06/19 16:32:32 UTC
[arrow-adbc] branch main updated: ci: add script to run clang-tidy (#801)
This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new eb451915 ci: add script to run clang-tidy (#801)
eb451915 is described below
commit eb45191566373bb5f5dbf440fe37502bb4d58b78
Author: David Li <li...@gmail.com>
AuthorDate: Mon Jun 19 12:32:27 2023 -0400
ci: add script to run clang-tidy (#801)
Fixes #780.
---
.clang-tidy | 24 ++++++++
.github/workflows/native-unix.yml | 35 ++++++++++++
c/driver/common/utils.c | 77 +++++++++++++-------------
c/driver/sqlite/sqlite.c | 3 +
ci/scripts/cpp_build.sh | 7 ++-
ci/scripts/{cpp_build.sh => cpp_clang_tidy.sh} | 66 +++++++++++-----------
6 files changed, 137 insertions(+), 75 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
new file mode 100644
index 00000000..ff9dbb0e
--- /dev/null
+++ b/.clang-tidy
@@ -0,0 +1,24 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+---
+# Only defaults for now, slowly enable more as desired
+# Disable clang-analyzer-core, it fires on nanoarrow (I think it doesn't see that ArrowBufferReserve won't leave buffer->data NULL)
+# Disable the warning about memset, etc. since it suggests C11 functions
+# Disable valist, it's buggy: https://github.com/llvm/llvm-project/issues/40656
+Checks: '-clang-analyzer-core.*,-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,-clang-analyzer-valist.Uninitialized'
+FormatStyle: google
+UseColor: true
diff --git a/.github/workflows/native-unix.yml b/.github/workflows/native-unix.yml
index c839cbbd..4eb8b8ab 100644
--- a/.github/workflows/native-unix.yml
+++ b/.github/workflows/native-unix.yml
@@ -188,6 +188,41 @@ jobs:
run: |
./ci/scripts/cpp_test.sh "$(pwd)/build"
+ clang-tidy-conda:
+ name: "clang-tidy"
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+ persist-credentials: false
+ - name: Get Date
+ id: get-date
+ shell: bash
+ run: |
+ echo "today=$(/bin/date -u '+%Y%m%d')" >> $GITHUB_OUTPUT
+ - name: Cache Conda
+ uses: actions/cache@v3
+ with:
+ path: ~/conda_pkgs_dir
+ key: conda-${{ runner.os }}-${{ steps.get-date.outputs.today }}-${{ env.CACHE_NUMBER }}-${{ hashFiles('ci/**') }}
+ - uses: conda-incubator/setup-miniconda@v2
+ with:
+ miniforge-variant: Mambaforge
+ miniforge-version: latest
+ use-only-tar-bz2: false
+ use-mamba: true
+ - name: Install Dependencies
+ shell: bash -l {0}
+ run: |
+ mamba install -c conda-forge \
+ --file ci/conda_env_cpp.txt
+
+ - name: clang-tidy
+ shell: bash -l {0}
+ run: |
+ ./ci/scripts/cpp_clang_tidy.sh "$(pwd)" "$(pwd)/build"
+
# ------------------------------------------------------------
# GLib/Ruby
# ------------------------------------------------------------
diff --git a/c/driver/common/utils.c b/c/driver/common/utils.c
index dbfe9613..dfac14f5 100644
--- a/c/driver/common/utils.c
+++ b/c/driver/common/utils.c
@@ -144,6 +144,7 @@ int StringBuilderAppend(struct StringBuilder* builder, const char* fmt, ...) {
va_start(argptr, fmt);
int ret = vsnprintf(builder->buffer + builder->size, n + 1, fmt, argptr);
if (ret < 0) {
+ va_end(argptr);
return errno;
}
@@ -429,7 +430,7 @@ AdbcStatusCode AdbcInitConnectionObjectsSchema(struct ArrowSchema* schema,
struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_view) {
struct AdbcGetObjectsData* get_objects_data =
- (struct AdbcGetObjectsData*)malloc(sizeof(struct AdbcGetObjectsData));
+ (struct AdbcGetObjectsData*)calloc(1, sizeof(struct AdbcGetObjectsData));
if (get_objects_data == NULL) {
return NULL;
}
@@ -488,8 +489,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
get_objects_data->fk_table_array = constraint_column_usage_items->children[2];
get_objects_data->fk_column_name_array = constraint_column_usage_items->children[3];
- get_objects_data->catalogs = (struct AdbcGetObjectsCatalog**)malloc(
- sizeof(struct AdbcGetObjectsCatalog*) * array_view->array->length);
+ get_objects_data->catalogs = (struct AdbcGetObjectsCatalog**)calloc(
+ array_view->array->length, sizeof(struct AdbcGetObjectsCatalog*));
if (get_objects_data->catalogs == NULL) {
goto error_handler;
@@ -497,10 +498,13 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
for (int64_t catalog_idx = 0; catalog_idx < array_view->array->length; catalog_idx++) {
struct AdbcGetObjectsCatalog* catalog =
- (struct AdbcGetObjectsCatalog*)malloc(sizeof(struct AdbcGetObjectsCatalog));
+ (struct AdbcGetObjectsCatalog*)calloc(1, sizeof(struct AdbcGetObjectsCatalog));
if (catalog == NULL) {
goto error_handler;
}
+ get_objects_data->catalogs[catalog_idx] = catalog;
+ get_objects_data->n_catalogs++;
+
catalog->n_db_schemas = 0;
catalog->catalog_name =
@@ -516,8 +520,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
if (db_schema_len == 0) {
catalog->catalog_db_schemas = NULL;
} else {
- catalog->catalog_db_schemas = (struct AdbcGetObjectsSchema**)malloc(
- sizeof(struct AdbcGetObjectsSchema*) * db_schema_len);
+ catalog->catalog_db_schemas = (struct AdbcGetObjectsSchema**)calloc(
+ db_schema_len, sizeof(struct AdbcGetObjectsSchema*));
if (catalog->catalog_db_schemas == NULL) {
goto error_handler;
}
@@ -525,10 +529,12 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
for (int64_t db_schema_index = db_schema_list_start;
db_schema_index < db_schema_list_end; db_schema_index++) {
struct AdbcGetObjectsSchema* schema =
- (struct AdbcGetObjectsSchema*)malloc(sizeof(struct AdbcGetObjectsSchema));
+ (struct AdbcGetObjectsSchema*)calloc(1, sizeof(struct AdbcGetObjectsSchema));
if (schema == NULL) {
goto error_handler;
}
+ catalog->catalog_db_schemas[db_schema_index - db_schema_list_start] = schema;
+ catalog->n_db_schemas++;
schema->n_db_schema_tables = 0;
schema->db_schema_name = ArrowArrayViewGetStringUnsafe(
@@ -542,19 +548,21 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
if (table_len == 0) {
schema->db_schema_tables = NULL;
} else {
- schema->db_schema_tables = (struct AdbcGetObjectsTable**)malloc(
- sizeof(struct AdbcGetObjectsTable*) * table_len);
+ schema->db_schema_tables = (struct AdbcGetObjectsTable**)calloc(
+ table_len, sizeof(struct AdbcGetObjectsTable*));
if (schema->db_schema_tables == NULL) {
goto error_handler;
}
for (int64_t table_index = table_list_start; table_index < table_list_end;
table_index++) {
- struct AdbcGetObjectsTable* table =
- (struct AdbcGetObjectsTable*)malloc(sizeof(struct AdbcGetObjectsTable));
+ struct AdbcGetObjectsTable* table = (struct AdbcGetObjectsTable*)calloc(
+ 1, sizeof(struct AdbcGetObjectsTable));
if (table == NULL) {
goto error_handler;
}
+ schema->db_schema_tables[table_index - table_list_start] = table;
+ schema->n_db_schema_tables++;
table->n_table_columns = 0;
table->n_table_constraints = 0;
@@ -573,8 +581,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
if (columns_len == 0) {
table->table_columns = NULL;
} else {
- table->table_columns = (struct AdbcGetObjectsColumn**)malloc(
- sizeof(struct AdbcGetObjectsColumn*) * columns_len);
+ table->table_columns = (struct AdbcGetObjectsColumn**)calloc(
+ columns_len, sizeof(struct AdbcGetObjectsColumn*));
if (table->table_columns == NULL) {
goto error_handler;
}
@@ -582,11 +590,13 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
for (int64_t column_index = columns_list_start;
column_index < columns_list_end; column_index++) {
struct AdbcGetObjectsColumn* column =
- (struct AdbcGetObjectsColumn*)malloc(
- sizeof(struct AdbcGetObjectsColumn));
+ (struct AdbcGetObjectsColumn*)calloc(
+ 1, sizeof(struct AdbcGetObjectsColumn));
if (column == NULL) {
goto error_handler;
}
+ table->table_columns[column_index - columns_list_start] = column;
+ table->n_table_columns++;
column->column_name = ArrowArrayViewGetStringUnsafe(
get_objects_data->column_name_array, column_index);
@@ -625,8 +635,6 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
get_objects_data->xdbc_is_autoincrement_array, column_index);
column->xdbc_is_generatedcolumn = ArrowArrayViewGetIntUnsafe(
get_objects_data->xdbc_is_generatedcolumn_array, column_index);
- table->table_columns[column_index - columns_list_start] = column;
- table->n_table_columns++;
}
}
@@ -639,8 +647,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
if (constraints_len == 0) {
table->table_constraints = NULL;
} else {
- table->table_constraints = (struct AdbcGetObjectsConstraint**)malloc(
- sizeof(struct AdbcGetObjectsConstraint*) * constraints_len);
+ table->table_constraints = (struct AdbcGetObjectsConstraint**)calloc(
+ constraints_len, sizeof(struct AdbcGetObjectsConstraint*));
if (table->table_constraints == NULL) {
goto error_handler;
}
@@ -648,11 +656,14 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
for (int64_t constraint_index = constraints_list_start;
constraint_index < constraints_list_end; constraint_index++) {
struct AdbcGetObjectsConstraint* constraint =
- (struct AdbcGetObjectsConstraint*)malloc(
- sizeof(struct AdbcGetObjectsConstraint));
+ (struct AdbcGetObjectsConstraint*)calloc(
+ 1, sizeof(struct AdbcGetObjectsConstraint));
if (constraint == NULL) {
goto error_handler;
}
+ table->table_constraints[constraint_index - constraints_list_start] =
+ constraint;
+ table->n_table_constraints++;
constraint->n_column_names = 0;
constraint->n_column_usages = 0;
@@ -671,8 +682,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
if (constraint_column_names_len == 0) {
constraint->constraint_column_names = NULL;
} else {
- constraint->constraint_column_names = (struct ArrowStringView*)malloc(
- sizeof(struct ArrowStringView) * constraint_column_names_len);
+ constraint->constraint_column_names = (struct ArrowStringView*)calloc(
+ constraint_column_names_len, sizeof(struct ArrowStringView));
if (constraint->constraint_column_names == NULL) {
goto error_handler;
}
@@ -702,9 +713,9 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
constraint->constraint_column_usages = NULL;
} else {
constraint->constraint_column_usages =
- (struct AdbcGetObjectsUsage**)malloc(
- sizeof(struct AdbcGetObjectsUsage*) *
- constraint_column_usages_len);
+ (struct AdbcGetObjectsUsage**)calloc(
+ constraint_column_usages_len,
+ sizeof(struct AdbcGetObjectsUsage*));
if (constraint->constraint_column_usages == NULL) {
goto error_handler;
}
@@ -714,8 +725,8 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
constraint_column_usage_index < constraint_column_usages_end;
constraint_column_usage_index++) {
struct AdbcGetObjectsUsage* usage =
- (struct AdbcGetObjectsUsage*)malloc(
- sizeof(struct AdbcGetObjectsUsage));
+ (struct AdbcGetObjectsUsage*)calloc(
+ 1, sizeof(struct AdbcGetObjectsUsage));
if (usage == NULL) {
goto error_handler;
}
@@ -738,22 +749,12 @@ struct AdbcGetObjectsData* AdbcGetObjectsDataInit(struct ArrowArrayView* array_v
constraint->n_column_usages++;
}
}
- table->table_constraints[constraint_index - constraints_list_start] =
- constraint;
- table->n_table_constraints++;
}
}
-
- schema->db_schema_tables[table_index - table_list_start] = table;
- schema->n_db_schema_tables++;
}
}
- catalog->catalog_db_schemas[db_schema_index - db_schema_list_start] = schema;
- catalog->n_db_schemas++;
}
}
- get_objects_data->catalogs[catalog_idx] = catalog;
- get_objects_data->n_catalogs++;
}
return get_objects_data;
diff --git a/c/driver/sqlite/sqlite.c b/c/driver/sqlite/sqlite.c
index 023c0762..4124098f 100644
--- a/c/driver/sqlite/sqlite.c
+++ b/c/driver/sqlite/sqlite.c
@@ -431,6 +431,7 @@ AdbcStatusCode SqliteConnectionGetColumnsImpl(
CHECK_NA(INTERNAL, ArrowArrayFinishElement(table_columns_items), error);
}
+ RAISE(INTERNAL, rc == SQLITE_DONE, sqlite3_errmsg(conn->conn), error);
return ADBC_STATUS_OK;
}
@@ -480,6 +481,7 @@ AdbcStatusCode SqliteConnectionGetConstraintsImpl(
.size_bytes = sqlite3_column_bytes(pk_stmt, 0)}),
error);
}
+ RAISE(INTERNAL, rc == SQLITE_DONE, sqlite3_errmsg(conn->conn), error);
if (has_primary_key) {
CHECK_NA(INTERNAL, ArrowArrayFinishElement(constraint_column_names_col), error);
CHECK_NA(INTERNAL, ArrowArrayAppendNull(constraint_column_usage_col, 1), error);
@@ -537,6 +539,7 @@ AdbcStatusCode SqliteConnectionGetConstraintsImpl(
error);
}
}
+ RAISE(INTERNAL, rc == SQLITE_DONE, sqlite3_errmsg(conn->conn), error);
if (prev_fk_id != -1) {
CHECK_NA(INTERNAL, ArrowArrayFinishElement(constraint_column_names_col), error);
CHECK_NA(INTERNAL, ArrowArrayFinishElement(constraint_column_usage_col), error);
diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh
index f7a37f6a..b5777dbc 100755
--- a/ci/scripts/cpp_build.sh
+++ b/ci/scripts/cpp_build.sh
@@ -50,17 +50,18 @@ build_subproject() {
pushd "${build_dir}"
set -x
+
cmake "${source_dir}/c" \
- "${ADBC_CMAKE_ARGS}" \
+ ${ADBC_CMAKE_ARGS} \
-DADBC_BUILD_SHARED="${ADBC_BUILD_SHARED}" \
+ -DADBC_BUILD_STATIC="${ADBC_BUILD_STATIC}" \
+ -DADBC_BUILD_TESTS="${ADBC_BUILD_TESTS}" \
-DADBC_DRIVER_MANAGER="${BUILD_DRIVER_MANAGER}" \
-DADBC_DRIVER_POSTGRESQL="${BUILD_DRIVER_POSTGRESQL}" \
-DADBC_DRIVER_SQLITE="${BUILD_DRIVER_SQLITE}" \
-DADBC_DRIVER_FLIGHTSQL="${BUILD_DRIVER_FLIGHTSQL}" \
-DADBC_DRIVER_SNOWFLAKE="${BUILD_DRIVER_SNOWFLAKE}" \
-DADBC_INTEGRATION_DUCKDB="${BUILD_INTEGRATION_DUCKDB}" \
- -DADBC_BUILD_STATIC="${ADBC_BUILD_STATIC}" \
- -DADBC_BUILD_TESTS="${ADBC_BUILD_TESTS}" \
-DADBC_USE_ASAN="${ADBC_USE_ASAN}" \
-DADBC_USE_UBSAN="${ADBC_USE_UBSAN}" \
-DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}" \
diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_clang_tidy.sh
similarity index 50%
copy from ci/scripts/cpp_build.sh
copy to ci/scripts/cpp_clang_tidy.sh
index f7a37f6a..16ffc26a 100755
--- a/ci/scripts/cpp_build.sh
+++ b/ci/scripts/cpp_clang_tidy.sh
@@ -24,11 +24,9 @@ set -e
: ${BUILD_DRIVER_SQLITE:=${BUILD_ALL}}
: ${BUILD_DRIVER_FLIGHTSQL:=${BUILD_ALL}}
: ${BUILD_DRIVER_SNOWFLAKE:=${BUILD_ALL}}
-# Must be explicitly enabled
-: ${BUILD_INTEGRATION_DUCKDB:=0}
: ${ADBC_BUILD_SHARED:=ON}
-: ${ADBC_BUILD_STATIC:=${BUILD_INTEGRATION_DUCKDB}}
+: ${ADBC_BUILD_STATIC:=OFF}
: ${ADBC_BUILD_TESTS:=ON}
: ${ADBC_USE_ASAN:=ON}
: ${ADBC_USE_UBSAN:=ON}
@@ -38,50 +36,50 @@ set -e
build_subproject() {
local -r source_dir="${1}"
- local -r build_dir="${2}"
- local -r install_dir="${3}"
-
- if [[ -z "${CMAKE_INSTALL_PREFIX}" ]]; then
- CMAKE_INSTALL_PREFIX="${install_dir}"
- fi
- echo "Installing to ${CMAKE_INSTALL_PREFIX}"
+ local -r build_dir="${2}/tidy"
mkdir -p "${build_dir}"
pushd "${build_dir}"
set -x
- cmake "${source_dir}/c" \
- "${ADBC_CMAKE_ARGS}" \
- -DADBC_BUILD_SHARED="${ADBC_BUILD_SHARED}" \
- -DADBC_DRIVER_MANAGER="${BUILD_DRIVER_MANAGER}" \
- -DADBC_DRIVER_POSTGRESQL="${BUILD_DRIVER_POSTGRESQL}" \
- -DADBC_DRIVER_SQLITE="${BUILD_DRIVER_SQLITE}" \
- -DADBC_DRIVER_FLIGHTSQL="${BUILD_DRIVER_FLIGHTSQL}" \
- -DADBC_DRIVER_SNOWFLAKE="${BUILD_DRIVER_SNOWFLAKE}" \
- -DADBC_INTEGRATION_DUCKDB="${BUILD_INTEGRATION_DUCKDB}" \
- -DADBC_BUILD_STATIC="${ADBC_BUILD_STATIC}" \
- -DADBC_BUILD_TESTS="${ADBC_BUILD_TESTS}" \
- -DADBC_USE_ASAN="${ADBC_USE_ASAN}" \
- -DADBC_USE_UBSAN="${ADBC_USE_UBSAN}" \
- -DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}" \
- -DCMAKE_INSTALL_LIBDIR=lib \
- -DCMAKE_INSTALL_PREFIX="${CMAKE_INSTALL_PREFIX}"
- set +x
- cmake --build . --target install -j
+ # XXX(https://github.com/llvm/llvm-project/issues/46804): expand
+ # CC and CXX symlinks
+ env CC=$(readlink -f ${CC:-$(which cc)}) \
+ CXX=$(readlink -f ${CXX:-$(which c++)}) \
+ cmake "${source_dir}/c" \
+ ${ADBC_CMAKE_ARGS} \
+ -DADBC_BUILD_SHARED="${ADBC_BUILD_SHARED}" \
+ -DADBC_BUILD_STATIC="${ADBC_BUILD_STATIC}" \
+ -DADBC_BUILD_TESTS="${ADBC_BUILD_TESTS}" \
+ -DADBC_DRIVER_MANAGER="${BUILD_DRIVER_MANAGER}" \
+ -DADBC_DRIVER_POSTGRESQL="${BUILD_DRIVER_POSTGRESQL}" \
+ -DADBC_DRIVER_SQLITE="${BUILD_DRIVER_SQLITE}" \
+ -DADBC_DRIVER_FLIGHTSQL="${BUILD_DRIVER_FLIGHTSQL}" \
+ -DADBC_DRIVER_SNOWFLAKE="${BUILD_DRIVER_SNOWFLAKE}" \
+ -DADBC_USE_ASAN="${ADBC_USE_ASAN}" \
+ -DADBC_USE_UBSAN="${ADBC_USE_UBSAN}" \
+ -DCMAKE_CXX_STANDARD=17 \
+ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
+ -DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}" \
+ -DCMAKE_INSTALL_LIBDIR=lib \
+ -DCMAKE_INSTALL_PREFIX="${CMAKE_INSTALL_PREFIX}"
+
+ clang-tidy \
+ -p "${build_dir}/compile_commands.json" \
+ --fix \
+ --quiet \
+ $(jq -r ".[] | .file" "${build_dir}/compile_commands.json")
+
+ set +x
popd
}
main() {
local -r source_dir="${1}"
local -r build_dir="${2}"
- local install_dir="${3}"
-
- if [[ -z "${install_dir}" ]]; then
- install_dir="${build_dir}/local"
- fi
- build_subproject "${source_dir}" "${build_dir}" "${install_dir}"
+ build_subproject "${source_dir}" "${build_dir}"
}
main "$@"