Conversation
|
@greptile |
Greptile SummaryThis PR adds comprehensive C language bindings to the zvec project, including a C API wrapper ( Key additions:
Critical issue:
The implementation is otherwise well-structured with thread-safe error handling, proper API versioning, and comprehensive functionality coverage. Confidence Score: 1/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class ZVec_C_API {
+zvec_initialize()
+zvec_shutdown()
+zvec_get_version()
}
class Collection_API {
+zvec_collection_create()
+zvec_collection_open()
+zvec_collection_insert()
+zvec_collection_query()
+zvec_collection_flush()
}
class Schema_API {
+zvec_collection_schema_create()
+zvec_field_schema_create()
+zvec_field_schema_set_hnsw_index()
+zvec_field_schema_set_invert_index()
}
class Document_API {
+zvec_doc_create()
+zvec_doc_set_pk()
+zvec_doc_add_field()
+zvec_doc_get_pk_copy()
}
class Index_API {
+zvec_index_params_hnsw_create()
+zvec_index_params_invert_create()
}
class Config_API {
+zvec_config_data_create()
+zvec_config_log_create()
}
class Memory_Management {
+zvec_free()
+zvec_free_string()
+zvec_free_str()
+zvec_docs_free()
}
ZVec_C_API --> Collection_API
ZVec_C_API --> Config_API
Collection_API --> Schema_API
Collection_API --> Document_API
Schema_API --> Index_API
Document_API --> Memory_Management
Collection_API --> Memory_Management
Last reviewed commit: 67ff247 |
src/c_api/c_api.cc
Outdated
| char *result = new char[pk.length() + 1]; | ||
| strcpy(result, pk.c_str()); | ||
| return result; |
There was a problem hiding this comment.
Memory allocated with new[] but documented (line 1966 in c_api.h) to be freed with free(). This causes undefined behavior - mixing new[]/delete[] with malloc/free.
| char *result = new char[pk.length() + 1]; | |
| strcpy(result, pk.c_str()); | |
| return result; | |
| char *result = static_cast<char*>(malloc(pk.length() + 1)); | |
| strcpy(result, pk.c_str()); | |
| return result; |
| ZVecString *zvec_string_create(const char *str) { | ||
| if (!str) { | ||
| set_last_error_details(ZVEC_ERROR_INVALID_ARGUMENT, | ||
| "String pointer cannot be null", __FILE__, __LINE__, | ||
| __FUNCTION__); | ||
| return nullptr; | ||
| } | ||
|
|
||
| ZVecString *zstr = nullptr; | ||
| char *data_buffer = nullptr; | ||
|
|
||
| try { | ||
| size_t len = strlen(str); | ||
| zstr = new ZVecString(); | ||
| data_buffer = new char[len + 1]; | ||
| strcpy(const_cast<char *>(data_buffer), str); | ||
|
|
||
| zstr->data = data_buffer; | ||
| zstr->length = len; | ||
| zstr->capacity = len + 1; | ||
|
|
||
| return zstr; | ||
|
|
||
| } catch (const std::exception &e) { | ||
| if (data_buffer) { | ||
| delete[] data_buffer; | ||
| } | ||
| if (zstr) { | ||
| delete zstr; | ||
| } | ||
|
|
||
| set_last_error_details(ZVEC_ERROR_INTERNAL_ERROR, | ||
| std::string("String creation failed: ") + e.what(), | ||
| __FILE__, __LINE__, __FUNCTION__); | ||
| return nullptr; | ||
| } | ||
| } |
There was a problem hiding this comment.
Memory allocated with new[] at line 152 but should be freed with delete[]. Since the API provides zvec_free_string which uses delete[], this is correct. However, zvec_string_create is also used in zvec_config_file_log_create where the memory is later freed with zvec_free_str (which calls free()) instead of delete[].
| ZVecString *zvec_string_create(const char *str) { | |
| if (!str) { | |
| set_last_error_details(ZVEC_ERROR_INVALID_ARGUMENT, | |
| "String pointer cannot be null", __FILE__, __LINE__, | |
| __FUNCTION__); | |
| return nullptr; | |
| } | |
| ZVecString *zstr = nullptr; | |
| char *data_buffer = nullptr; | |
| try { | |
| size_t len = strlen(str); | |
| zstr = new ZVecString(); | |
| data_buffer = new char[len + 1]; | |
| strcpy(const_cast<char *>(data_buffer), str); | |
| zstr->data = data_buffer; | |
| zstr->length = len; | |
| zstr->capacity = len + 1; | |
| return zstr; | |
| } catch (const std::exception &e) { | |
| if (data_buffer) { | |
| delete[] data_buffer; | |
| } | |
| if (zstr) { | |
| delete zstr; | |
| } | |
| set_last_error_details(ZVEC_ERROR_INTERNAL_ERROR, | |
| std::string("String creation failed: ") + e.what(), | |
| __FILE__, __LINE__, __FUNCTION__); | |
| return nullptr; | |
| } | |
| } | |
| ZVecString *zvec_string_create(const char *str) { | |
| if (!str) { | |
| set_last_error_details(ZVEC_ERROR_INVALID_ARGUMENT, | |
| "String pointer cannot be null", __FILE__, __LINE__, | |
| __FUNCTION__); | |
| return nullptr; | |
| } | |
| ZVecString *zstr = nullptr; | |
| char *data_buffer = nullptr; | |
| try { | |
| size_t len = strlen(str); | |
| zstr = new ZVecString(); | |
| data_buffer = static_cast<char*>(malloc(len + 1)); | |
| strcpy(data_buffer, str); | |
| zstr->data = data_buffer; | |
| zstr->length = len; | |
| zstr->capacity = len + 1; | |
| return zstr; | |
| } catch (const std::exception &e) { | |
| if (data_buffer) { | |
| free(data_buffer); | |
| } | |
| if (zstr) { | |
| delete zstr; | |
| } | |
| set_last_error_details(ZVEC_ERROR_INTERNAL_ERROR, | |
| std::string("String creation failed: ") + e.what(), | |
| __FILE__, __LINE__, __FUNCTION__); | |
| return nullptr; | |
| } | |
| } |
format c_api.h fix some code format c_api code build rpm fix some code fix some code remove rpm build Add GitHub Actions release workflow for C API add release linux-arm64 remove c api version fix some code fix some code fix some code refact some code format some code format some code format some code add api reference
| // Version Information | ||
| // ============================================================================= | ||
|
|
||
| /** @brief Major version number */ |
There was a problem hiding this comment.
Prefer to git tag based version to manually maintained version.
在 C 项目中,基于 Git Tag 自动生成版本号是推荐的最佳实践。这样可以确保代码版本与 Git 标签保持同步,避免手动修改头文件的繁琐。
实现这一功能通常分为三个步骤:
- 创建版本头文件模板 (
version.h.in)。 - 在
CMakeLists.txt中编写脚本:获取 Git Tag 并解析。 - 生成头文件:在编译前通过 CMake 填充模板。
以下是完整实现方案:
1. 创建头文件模板 version.h.in
在项目目录下创建一个名为 version.h.in 的文件。使用 @VAR@ 语法作为占位符:
#ifndef ZVEC_VERSION_H
#define ZVEC_VERSION_H
/** @brief Major version number */
#define ZVEC_VERSION_MAJOR @ZVEC_VERSION_MAJOR@
/** @brief Minor version number */
#define ZVEC_VERSION_MINOR @ZVEC_VERSION_MINOR@
/** @brief Patch version number */
#define ZVEC_VERSION_PATCH @ZVEC_VERSION_PATCH@
/** @brief Full version string */
#define ZVEC_VERSION_STRING "@ZVEC_VERSION_STRING@"
#endif // ZVEC_VERSION_H2. 在 CMakeLists.txt 中获取 Git 信息
编写逻辑来运行 git describe 并解析版本号。
cmake_minimum_required(VERSION 3.10)
project(zvec LANGUAGES C)
# --- 1. 获取 Git 版本信息 ---
# 默认版本号(如果不在 git 仓库中或没找到 tag)
set(ZVEC_VERSION_MAJOR 0)
set(ZVEC_VERSION_MINOR 0)
set(ZVEC_VERSION_PATCH 0)
set(ZVEC_VERSION_STRING "0.0.0-unknown")
# 尝试运行 git describe
find_package(Git QUIET)
if(GIT_FOUND AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/.git")
execute_process(
COMMAND ${GIT_EXECUTABLE} describe --tags --always --dirty
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
OUTPUT_VARIABLE GIT_TAG
OUTPUT_STRIP_TRAILING_WHITESPACE
)
# 假设 tag 格式为 v1.2.3 或 1.2.3
# 使用正则解析:^v?([0-9]+)\.([0-9]+)\.([0-9]+)
if(GIT_TAG MATCHES "^v?([0-9]+)\\.([0-9]+)\\.([0-9]+)")
set(ZVEC_VERSION_MAJOR ${CMAKE_MATCH_1})
set(ZVEC_VERSION_MINOR ${CMAKE_MATCH_2})
set(ZVEC_VERSION_PATCH ${CMAKE_MATCH_3})
set(ZVEC_VERSION_STRING ${GIT_TAG})
else()
# 如果 tag 不符合语义化版本格式,则直接使用整个 tag 作为字符串
set(ZVEC_VERSION_STRING ${GIT_TAG})
endif()
endif()
# --- 2. 生成版本头文件 ---
# 将模板填充后生成到 build 目录,防止污染源码目录
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/version.h.in"
"${CMAKE_CURRENT_BINARY_DIR}/generated/zvec_version.h"
)
# --- 3. 配置项目 ---
add_library(zvec src/zvec.c)
# 必须将生成的目录加入包含路径
target_include_directories(zvec PUBLIC
"${CMAKE_CURRENT_BINARY_DIR}/generated"
)3. 在 C 代码中使用
在你的代码中,只需包含生成的头文件:
#include "zvec_version.h"
#include <stdio.h>
void print_version() {
printf("ZVec Version: %s\n", ZVEC_VERSION_STRING);
printf("Major: %d, Minor: %d, Patch: %d\n",
ZVEC_VERSION_MAJOR, ZVEC_VERSION_MINOR, ZVEC_VERSION_PATCH);
}4. 工作流程说明
- 打标签:在 Git 中打一个符合
vX.Y.Z格式的标签:git tag -a v0.3.0 -m "Release version 0.3.0" - 运行 CMake:
此时 CMake 会执行
mkdir build && cd build cmake ..
git describe,获取到v0.3.0,解析出 0, 3, 0,并生成zvec_version.h。 - 自动处理开发版本:
如果你在v0.3.0之后又提交了 5 次代码,git describe --tags --dirty会输出类似v0.3.0-5-gabc123的字符串,这会自动变成你的ZVEC_VERSION_STRING。
进阶:如何让版本号在每次 build 时更新?
上述方案只在执行 cmake 配置时更新版本号。如果你希望每次 make 时都检查版本(即使没有重新 run cmake),通常需要使用 add_custom_command 调用一个独立的 CMake 脚本来生成头文件。
但对于大多数项目,只有在发布新版本(打 Tag)时才需要改变版本号,因此上述 configure_file 方案已经足够,且性能更好。如果你修改了 Tag,只需手动运行一次 cmake .. 即可。
|
|
||
| // RAII guard for array allocated with new[] | ||
| template <typename T> | ||
| struct DeleteArrayGuard { |
There was a problem hiding this comment.
Those guards are not used? Can they be removed?
BTW, you can used unique_ptr instead of Delete(Array)Guard.
|
|
||
| bool zvec_check_version(int major, int minor, int patch) { | ||
| if (major < 0 || minor < 0 || patch < 0) { | ||
| set_last_error_details(ZVEC_ERROR_INVALID_ARGUMENT, |
There was a problem hiding this comment.
define macro to simplify code
#define SET_LAST_ERROR(code, msg) set_last_error_details(code, msg, __FILE__,
__LINE__, __FUNCTION__);| /** | ||
| * @brief Mutable string structure (owns memory) | ||
| */ | ||
| typedef struct { |
There was a problem hiding this comment.
Use c99 flexible array to reduce malloc count(2->1)
struct A {
int a;
char data[]; // flexible array member
};
| set_last_error(status.message()); | ||
| return ZVEC_ERROR_INTERNAL_ERROR; | ||
| } | ||
| } g_initialized.store(true); |
| cpp_config.log_config = log_config; | ||
| } | ||
| // Initialize global configuration | ||
| auto status = zvec::GlobalConfig::Instance().Initialize(cpp_config); |
There was a problem hiding this comment.
Initialize and error check common logic could be extracted out of if/else.
| return ZVEC_OK;) | ||
| } | ||
|
|
||
| ZVecErrorCode zvec_is_initialized(bool *initialized) { |
There was a problem hiding this comment.
Update to bool zvec_is_initialized() to simplify usage?
|
|
||
| void zvec_config_file_log_destroy(ZVecFileLogConfig *config) { | ||
| if (config) { | ||
| if (config->dir.data) free((void *)config->dir.data); |
There was a problem hiding this comment.
cast is unnecessary, void * can be implicitly casted.
| } | ||
|
|
||
| void zvec_config_console_log_destroy(ZVecConsoleLogConfig *config) { | ||
| if (config) { |
There was a problem hiding this comment.
It's unnecessary. free accepts NULL.
| static char *copy_string(const std::string &str) { | ||
| if (str.empty()) return nullptr; | ||
| char *copy = static_cast<char *>(malloc(str.length() + 1)); | ||
| strcpy(copy, str.c_str()); |
There was a problem hiding this comment.
Use strncpy to support binary str input.
| for (size_t i = 0; i < *result_count; ++i) { | ||
| const std::string pk = i < pks.size() ? pks[i] : std::string(); | ||
| const std::string message = statuses[i].message(); | ||
| (*results)[i].pk = copy_string(pk); |
There was a problem hiding this comment.
Can we use ordered style so that pk does not need to be stored in result?
| ZVecDoc *zvec_doc_create(void) { | ||
| ZVEC_TRY_RETURN_NULL("Failed to create document", { | ||
| auto doc_ptr = | ||
| new std::shared_ptr<zvec::Doc>(std::make_shared<zvec::Doc>()); |
There was a problem hiding this comment.
Could we just use plain Doc?
auto* doc = new Doc();
| set_last_error("Invalid value size for vector_fp64 type"); | ||
| return error_code; | ||
| } | ||
| (*doc_ptr)->set(name, vec); |
There was a problem hiding this comment.
move(vec) for better performance
| reinterpret_cast<const int8_t *>(field->value.vector_value.data), | ||
| reinterpret_cast<const int8_t *>(field->value.vector_value.data) + | ||
| field->value.vector_value.length); | ||
| (*doc_ptr)->set(name, vec); |
There was a problem hiding this comment.
move(vec) for better performance
Summary
Add C language bindings.
Changes
src/c_api/c_api.ccsrc/include/zvec/c_api.hexamples/c_api/Testing
tests/c_api/c_api_test.c