From 3e16a23768680d95090e5c1d75a19395980e0a91 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 23 Oct 2024 19:48:31 -0500 Subject: [PATCH 1/4] T5528: add adapter interface Expose legacy set/delete functions in a form suitable for external calls. --- Makefile.am | 2 + src/vy_adapter.cpp | 102 +++++++++++++++++++++++++++++++++++++++++++++ src/vy_adapter.h | 31 ++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 src/vy_adapter.cpp create mode 100644 src/vy_adapter.h diff --git a/Makefile.am b/Makefile.am index 2170f95a..6ef8d643 100644 --- a/Makefile.am +++ b/Makefile.am @@ -52,6 +52,7 @@ src_libvyatta_cfg_la_SOURCES += src/cnode/cnode-algorithm.cpp src_libvyatta_cfg_la_SOURCES += src/cparse/cparse.cpp src_libvyatta_cfg_la_SOURCES += src/cparse/cparse_lex.c src_libvyatta_cfg_la_SOURCES += src/commit/commit-algorithm.cpp +src_libvyatta_cfg_la_SOURCES += src/vy_adapter.cpp CLEANFILES = src/cli_parse.c src/cli_parse.h src/cli_def.c src/cli_val.c CLEANFILES += src/cparse/cparse.cpp src/cparse/cparse.h CLEANFILES += src/cparse/cparse_lex.c @@ -61,6 +62,7 @@ LDADD += $(GOBJECT_LIBS) vincludedir = $(includedir)/vyatta-cfg vinclude_HEADERS = src/cli_cstore.h +vinclude_HEADERS += src/vy_adapter.h vcincdir = $(vincludedir)/cstore vcinc_HEADERS = src/cstore/cstore-c.h diff --git a/src/vy_adapter.cpp b/src/vy_adapter.cpp new file mode 100644 index 00000000..793728fe --- /dev/null +++ b/src/vy_adapter.cpp @@ -0,0 +1,102 @@ +/* Copyright 2024 VyOS maintainers and contributors + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace cstore; + +static uint64_t uint_of_voidptr(void* p) +{ + assert (((uintptr_t) p & 1) == 0); + return (uint64_t) p | 1; +} + +static void *voidptr_of_uint(uint64_t v) +{ + return (void *)(uintptr_t)(v & ~1); +} + +static Cstore *cstore_of_handle(uint64_t handle) +{ + return (Cstore *) voidptr_of_uint(handle); +} + +uint64_t +vy_cstore_init(void) +{ + Cstore *handle = Cstore::createCstore(false); + return uint_of_voidptr(handle); +} + +void +vy_cstore_free(uint64_t handle) +{ + Cstore *h = cstore_of_handle(handle); + delete h; +} + +int +vy_in_session(uint64_t handle) +{ + Cstore *h = cstore_of_handle(handle); + return h->inSession() ? 1 : 0; +} + +int +vy_set_path(uint64_t handle, const void** path_ptr, size_t len) +{ + Cstore *cstore = cstore_of_handle(handle); + const char **path = (const char **) path_ptr; + Cpath path_comps = Cpath(path, len); + int res; + + res = cstore->validateSetPath(path_comps); + if (!res) { + return 1; + } + + res = cstore->setCfgPath(path_comps); + if (!res) { + return 2; + } + + return 0; +} + +int +vy_delete_path(uint64_t handle, const void** path_ptr, size_t len) +{ + Cstore *cstore = cstore_of_handle(handle); + const char **path = (const char **) path_ptr; + Cpath path_comps = Cpath(path, len); + int res; + + res = cstore->deleteCfgPath(path_comps); + if (!res) { + return 1; + } + + return 0; +} diff --git a/src/vy_adapter.h b/src/vy_adapter.h new file mode 100644 index 00000000..8e0f376d --- /dev/null +++ b/src/vy_adapter.h @@ -0,0 +1,31 @@ +/* Copyright 2024 VyOS maintainers and contributors + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#ifndef _VY_ADAPTER_H_ +#define _VY_ADAPTER_H_ +#ifdef __cplusplus +extern "C" { +#endif + +uint64_t vy_cstore_init(void); +void vy_cstore_free(uint64_t); +int vy_in_session(uint64_t); +int vy_set_path(uint64_t, const void **, size_t); +int vy_delete_path(uint64_t, const void **, size_t); + +#ifdef __cplusplus +} +#endif +#endif From 9d4787256f6b300927e7892047313d725787d828 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 23 Oct 2024 19:48:31 -0500 Subject: [PATCH 2/4] T6717: add redirect_stdout Legacy set/delete methods write errors to stdout; capture for redirection in return value. --- src/vy_adapter.cpp | 104 ++++++++++++++++++++++++++++++++++++++++++--- src/vy_adapter.h | 4 +- 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/src/vy_adapter.cpp b/src/vy_adapter.cpp index 793728fe..dd66dfb4 100644 --- a/src/vy_adapter.cpp +++ b/src/vy_adapter.cpp @@ -21,12 +21,83 @@ #include #include #include +#include +#include #include #include using namespace cstore; +enum PIPES { READ, WRITE }; + +extern FILE *out_stream; + +class stdout_redirect { + public: + stdout_redirect(): old_stdout(0), redirecting(false) { + out_pipe[READ] = 0; + out_pipe[WRITE] = 0; + if (pipe(out_pipe) == -1) { + return; + } + + old_stdout = dup(fileno(stdout)); + setbuf(stdout, NULL); + dup2(out_pipe[WRITE], fileno(stdout)); + + redirecting = true; + } + + std::string get_redirected_output() { + if (!redirecting) { + return ""; + } + + char buffer[1024]; + ssize_t bytesRead = 0; + std::string redirected_output = "\n"; + fcntl(out_pipe[READ], F_SETFL, O_NONBLOCK); + while ((bytesRead = read(out_pipe[READ], buffer, sizeof(buffer))) > 0) { + redirected_output.append(buffer, bytesRead); + } + + return redirected_output; + } + + ~stdout_redirect() { + if (!redirecting) { + return; + } + + dup2(old_stdout, fileno(stdout)); + + if (old_stdout > 0) { + close(old_stdout); + } + if (out_pipe[READ] > 0) { + close(out_pipe[READ]); + } + if (out_pipe[WRITE] > 0) { + close(out_pipe[WRITE]); + } + } + + private: + int out_pipe[2]; + int old_stdout; + bool redirecting; +}; + +const char *out_data_copy(std::string msg) +{ + size_t len = msg.length(); + char *out_data = (char *) malloc(len + 1); + msg.copy(out_data, len); + out_data[len] = '\0'; + return out_data; +} + static uint64_t uint_of_voidptr(void* p) { assert (((uintptr_t) p & 1) == 0); @@ -64,39 +135,58 @@ vy_in_session(uint64_t handle) return h->inSession() ? 1 : 0; } -int +const char * vy_set_path(uint64_t handle, const void** path_ptr, size_t len) { Cstore *cstore = cstore_of_handle(handle); const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); + const char *out_data; + std::string out_str = ""; int res; + out_stream = stdout; + stdout_redirect redirect = stdout_redirect(); + res = cstore->validateSetPath(path_comps); if (!res) { - return 1; + out_str = "\nInvalid set path: " + path_comps.to_string() + "\n"; + out_str.append(redirect.get_redirected_output()); + goto out; } res = cstore->setCfgPath(path_comps); if (!res) { - return 2; + out_str = "\nSet config path failed: " + path_comps.to_string() + "\n"; + out_str.append(redirect.get_redirected_output()); } - return 0; +out: + out_data = out_data_copy(out_str); + out_stream = NULL; + return out_data; } -int +const char * vy_delete_path(uint64_t handle, const void** path_ptr, size_t len) { Cstore *cstore = cstore_of_handle(handle); const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); + const char *out_data; + std::string out_str = ""; int res; + out_stream = stdout; + stdout_redirect redirect = stdout_redirect(); + res = cstore->deleteCfgPath(path_comps); if (!res) { - return 1; + out_str = "\nDelete failed: " + path_comps.to_string() + "\n"; + out_str.append(redirect.get_redirected_output()); } - return 0; + out_data = out_data_copy(out_str); + out_stream = NULL; + return out_data; } diff --git a/src/vy_adapter.h b/src/vy_adapter.h index 8e0f376d..fba6368c 100644 --- a/src/vy_adapter.h +++ b/src/vy_adapter.h @@ -22,8 +22,8 @@ extern "C" { uint64_t vy_cstore_init(void); void vy_cstore_free(uint64_t); int vy_in_session(uint64_t); -int vy_set_path(uint64_t, const void **, size_t); -int vy_delete_path(uint64_t, const void **, size_t); +const char *vy_set_path(uint64_t, const void **, size_t); +const char *vy_delete_path(uint64_t, const void **, size_t); #ifdef __cplusplus } From 39ec15bd84a5388181980574698096f9029f45de Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 23 Oct 2024 19:48:31 -0500 Subject: [PATCH 3/4] T6717: split validate and set functions Separate the application of validators from the cstore update. --- src/vy_adapter.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++++++ src/vy_adapter.h | 2 ++ 2 files changed, 50 insertions(+) diff --git a/src/vy_adapter.cpp b/src/vy_adapter.cpp index dd66dfb4..49e186dc 100644 --- a/src/vy_adapter.cpp +++ b/src/vy_adapter.cpp @@ -135,6 +135,30 @@ vy_in_session(uint64_t handle) return h->inSession() ? 1 : 0; } +const char * +vy_validate_path(uint64_t handle, const void** path_ptr, size_t len) +{ + Cstore *cstore = cstore_of_handle(handle); + const char **path = (const char **) path_ptr; + Cpath path_comps = Cpath(path, len); + const char *out_data; + std::string out_str = ""; + int res; + + out_stream = stdout; + stdout_redirect redirect = stdout_redirect(); + + res = cstore->validateSetPath(path_comps); + if (!res) { + out_str = "\nInvalid set path: " + path_comps.to_string() + "\n"; + out_str.append(redirect.get_redirected_output()); + } + + out_data = out_data_copy(out_str); + out_stream = NULL; + return out_data; +} + const char * vy_set_path(uint64_t handle, const void** path_ptr, size_t len) { @@ -148,6 +172,30 @@ vy_set_path(uint64_t handle, const void** path_ptr, size_t len) out_stream = stdout; stdout_redirect redirect = stdout_redirect(); + res = cstore->setCfgPath(path_comps); + if (!res) { + out_str = "\nSet config path failed: " + path_comps.to_string() + "\n"; + out_str.append(redirect.get_redirected_output()); + } + + out_data = out_data_copy(out_str); + out_stream = NULL; + return out_data; +} + +const char * +vy_legacy_set_path(uint64_t handle, const void** path_ptr, size_t len) +{ + Cstore *cstore = cstore_of_handle(handle); + const char **path = (const char **) path_ptr; + Cpath path_comps = Cpath(path, len); + const char *out_data; + std::string out_str = ""; + int res; + + out_stream = stdout; + stdout_redirect redirect = stdout_redirect(); + res = cstore->validateSetPath(path_comps); if (!res) { out_str = "\nInvalid set path: " + path_comps.to_string() + "\n"; diff --git a/src/vy_adapter.h b/src/vy_adapter.h index fba6368c..636c303a 100644 --- a/src/vy_adapter.h +++ b/src/vy_adapter.h @@ -22,7 +22,9 @@ extern "C" { uint64_t vy_cstore_init(void); void vy_cstore_free(uint64_t); int vy_in_session(uint64_t); +const char *vy_validate_path(uint64_t, const void **, size_t); const char *vy_set_path(uint64_t, const void **, size_t); +const char *vy_legacy_set_path(uint64_t, const void **, size_t); const char *vy_delete_path(uint64_t, const void **, size_t); #ifdef __cplusplus From a428d47a9fe2a5d18685464e28a6d302da5133a8 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 23 Oct 2024 19:48:31 -0500 Subject: [PATCH 4/4] T6717: memory management of alloced return values The ctypes binding makes a copy of the string return value; the originating C memory allocation needs to be explicitly managed. --- src/vy_adapter.cpp | 50 +++++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/src/vy_adapter.cpp b/src/vy_adapter.cpp index 49e186dc..cf2f50e3 100644 --- a/src/vy_adapter.cpp +++ b/src/vy_adapter.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include #include @@ -89,10 +91,22 @@ class stdout_redirect { bool redirecting; }; -const char *out_data_copy(std::string msg) +struct cstore_obj { + Cstore *cstore; + std::list output; +}; + +static struct cstore_obj* create_handle() { + struct cstore_obj *h = new cstore_obj; + h->cstore = Cstore::createCstore(false); + return h; +}; + +const char *out_data_copy(std::string msg, cstore_obj *o) { size_t len = msg.length(); char *out_data = (char *) malloc(len + 1); + o->output.push_back(out_data); msg.copy(out_data, len); out_data[len] = '\0'; return out_data; @@ -109,36 +123,41 @@ static void *voidptr_of_uint(uint64_t v) return (void *)(uintptr_t)(v & ~1); } -static Cstore *cstore_of_handle(uint64_t handle) +static cstore_obj *cstore_obj_of_handle(uint64_t handle) { - return (Cstore *) voidptr_of_uint(handle); + return (cstore_obj *) voidptr_of_uint(handle); } uint64_t vy_cstore_init(void) { - Cstore *handle = Cstore::createCstore(false); + cstore_obj *handle = create_handle(); return uint_of_voidptr(handle); } void vy_cstore_free(uint64_t handle) { - Cstore *h = cstore_of_handle(handle); + cstore_obj *h = cstore_obj_of_handle(handle); + for (char * x: h->output) { + free(x); + } + delete h->cstore; delete h; } int vy_in_session(uint64_t handle) { - Cstore *h = cstore_of_handle(handle); + Cstore *h = cstore_obj_of_handle(handle)->cstore; return h->inSession() ? 1 : 0; } const char * vy_validate_path(uint64_t handle, const void** path_ptr, size_t len) { - Cstore *cstore = cstore_of_handle(handle); + cstore_obj *obj = cstore_obj_of_handle(handle); + Cstore *cstore = obj->cstore; const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); const char *out_data; @@ -154,7 +173,7 @@ vy_validate_path(uint64_t handle, const void** path_ptr, size_t len) out_str.append(redirect.get_redirected_output()); } - out_data = out_data_copy(out_str); + out_data = out_data_copy(out_str, obj); out_stream = NULL; return out_data; } @@ -162,7 +181,8 @@ vy_validate_path(uint64_t handle, const void** path_ptr, size_t len) const char * vy_set_path(uint64_t handle, const void** path_ptr, size_t len) { - Cstore *cstore = cstore_of_handle(handle); + cstore_obj *obj = cstore_obj_of_handle(handle); + Cstore *cstore = obj->cstore; const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); const char *out_data; @@ -178,7 +198,7 @@ vy_set_path(uint64_t handle, const void** path_ptr, size_t len) out_str.append(redirect.get_redirected_output()); } - out_data = out_data_copy(out_str); + out_data = out_data_copy(out_str, obj); out_stream = NULL; return out_data; } @@ -186,7 +206,8 @@ vy_set_path(uint64_t handle, const void** path_ptr, size_t len) const char * vy_legacy_set_path(uint64_t handle, const void** path_ptr, size_t len) { - Cstore *cstore = cstore_of_handle(handle); + cstore_obj *obj = cstore_obj_of_handle(handle); + Cstore *cstore = obj->cstore; const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); const char *out_data; @@ -210,7 +231,7 @@ vy_legacy_set_path(uint64_t handle, const void** path_ptr, size_t len) } out: - out_data = out_data_copy(out_str); + out_data = out_data_copy(out_str, obj); out_stream = NULL; return out_data; } @@ -218,7 +239,8 @@ vy_legacy_set_path(uint64_t handle, const void** path_ptr, size_t len) const char * vy_delete_path(uint64_t handle, const void** path_ptr, size_t len) { - Cstore *cstore = cstore_of_handle(handle); + cstore_obj *obj = cstore_obj_of_handle(handle); + Cstore *cstore = obj->cstore; const char **path = (const char **) path_ptr; Cpath path_comps = Cpath(path, len); const char *out_data; @@ -234,7 +256,7 @@ vy_delete_path(uint64_t handle, const void** path_ptr, size_t len) out_str.append(redirect.get_redirected_output()); } - out_data = out_data_copy(out_str); + out_data = out_data_copy(out_str, obj); out_stream = NULL; return out_data; }