Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
lidaobing
marked this pull request as draft
July 26, 2026 23:42
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new GObject-based iptux-gi shared library intended for GObject Introspection (GI) bindings, and wires it into the Meson build so the Iptux namespace can be generated/installed. It also adds a CoreThread::Ptr alias to simplify sharing CoreThread instances across components.
Changes:
- Add new
src/iptux-gisubproject that builds and installslibiptux-giand generates a GIR for namespaceIptux. - Introduce
IptuxService(aGObject) plus a constructor helperiptux_service_new(). - Expose
CoreThread::Ptras astd::shared_ptr<CoreThread>alias in the public API header.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| meson.build | Treats the project as both C and C++ to support new GI-related build steps. |
| src/meson.build | Adds the new iptux-gi subdirectory to the build. |
| src/iptux-gi/meson.build | Builds/install iptux-gi and configures GIR generation. |
| src/iptux-gi/iptux-service.h | Declares the new IptuxService GObject type and constructor. |
| src/iptux-gi/iptux-service.cpp | Implements the IptuxService type and iptux_service_new(). |
| src/api/iptux-core/CoreThread.h | Adds CoreThread::Ptr shared-pointer alias. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
src/iptux-gi/iptux-service.cpp:9
- GObject instances are allocated as plain C memory; embedding a C++ std::shared_ptr (CoreThread::Ptr) directly in the instance struct means its constructor/destructor will never run, causing undefined behavior (and the current
self->core_thread = 0;also won’t compile for std::shared_ptr). Store a pointer to a heap-allocated shared_ptr and free it in finalize (or use placement-new + explicit destruction).
struct _IptuxService {
GObject parent_instance;
CoreThread::Ptr core_thread;
};
src/iptux-gi/meson.build:5
- The iptux-gi sources include iptux-core headers that pull in <gio/gio.h> and <json/json.h> (via CoreThread.h and IptuxConfig.h). Without gio-2.0 and jsoncpp dependencies here, the build will fail due to missing include paths.
gobject_dep = dependency('gobject-2.0')
sigc_dep = dependency('sigc++-2.0')
dependencies = [gobject_dep, sigc_dep]
Comment on lines
+5
to
+9
| G_BEGIN_DECLS | ||
|
|
||
| #define IPTUX_TYPE_SERVICE (iptux_service_get_type()) | ||
| G_DECLARE_FINAL_TYPE(IptuxService, iptux_service, IPTUX, SERVICE, GObject) | ||
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (4)
src/iptux-gi/iptux-service.h:10
G_DECLARE_FINAL_TYPEalready definesIPTUX_TYPE_SERVICE; the extra#define IPTUX_TYPE_SERVICE (iptux_service_get_type())will cause a macro redefinition warning (and this project builds withwerror=true). Remove the redundant macro definition.
G_BEGIN_DECLS
#define IPTUX_TYPE_SERVICE (iptux_service_get_type())
G_DECLARE_FINAL_TYPE(IptuxService, iptux_service, IPTUX, SERVICE, GObject)
src/iptux-gi/iptux-config.h:9
G_DECLARE_FINAL_TYPEalready definesIPTUX_TYPE_CONFIG; the extra#define IPTUX_TYPE_CONFIG (iptux_config_get_type())will cause a macro redefinition warning (and this project builds withwerror=true). Remove the redundant macro definition.
G_BEGIN_DECLS
#define IPTUX_TYPE_CONFIG (iptux_config_get_type())
G_DECLARE_FINAL_TYPE(IptuxConfig, iptux_config, IPTUX, CONFIG, GObject)
src/iptux-gi/iptux-service.cpp:21
std::shared_ptris a C++ type with a non-trivial constructor/destructor, but GObject instances are allocated/freed as plain C memory (constructors/destructors are not run). StoringCoreThread::Ptrdirectly inside the instance struct leads to undefined behavior and likely leaks/crashes. Use a pointer to a heap-allocated C++ object (and delete it infinalize), or store the C++ object in a separatelynew'd private struct whose lifetime you manage ininit/finalize.
struct _IptuxService {
GObject parent_instance;
CoreThread::Ptr core_thread;
::IptuxConfig* config;
};
src/iptux-gi/iptux-config.cpp:13
self->configis aiptux::IptuxConfig::PtrC++ member (declared iniptux-priv.h), but GObject allocates instances as raw C memory, so thestd::shared_ptris never constructed. Assigningnullptriniptux_config_init()is undefined behavior. Additionally,iptux_config_new()never creates an underlyingiptux::IptuxConfig, so any consumer expecting a usable config will crash. Store a pointer to a heap-allocatediptux::IptuxConfig::Ptr(or another manually-managed C++ object) and initialize it to a valid config (e.g.IptuxConfig::newFromString("{}")).
static void iptux_config_init(IptuxConfig* self) {
self->config = nullptr;
}
Comment on lines
+6
to
+9
| struct _IptuxConfig { | ||
| GObject parent_instance; | ||
| iptux::IptuxConfig::Ptr config; | ||
| }; No newline at end of file |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (6)
src/iptux-gi/iptux-config.cpp:13
IptuxConfigstores astd::shared_ptrinside a GObject instance struct, but the shared_ptr is never constructed/destructed (GObject allocates instance memory without running C++ ctors/dtors). Also,iptux_config_new_from_fname()passes a filename toIptuxConfig::newFromString(), which expects JSON content and will not load the file.
static void iptux_config_init(IptuxConfig* self) {
self->config = nullptr;
}
src/iptux-gi/iptux-service.cpp:4
IptuxServiceembeds astd::shared_ptrin the GObject instance struct; since GObject doesn't run C++ constructors, the shared_ptr needs explicit placement-new construction before it’s assigned later.
#include "iptux-service.h"
#include "iptux-core/CoreThread.h"
#include "iptux-priv.h"
src/iptux-gi/iptux-service.cpp:34
- The
configconstruct-only property is declared as a raw pointer and read viag_value_get_pointer(). This loses type-safety and is not introspectable (GIR will see a gpointer). Use an object property (g_param_spec_object) and retrieve it viag_value_get_object().
switch (property_id) {
case PROP_CONFIG:
self->config = static_cast<::IptuxConfig*>(g_value_get_pointer(value));
break;
src/iptux-gi/iptux-service.cpp:69
iptux_service_init()assigns tocore_thread, but the shared_ptr object was never constructed (GObject instance memory is just zeroed). Use placement-new to construct the shared_ptr, and initializeconfigto nullptr sog_set_object()is safe.
static void iptux_service_init(IptuxService* self) {
self->core_thread = 0;
}
src/iptux-gi/meson.build:6
- This target includes
iptux-core/CoreThread.h(which includes<gio/gio.h>), but the Meson target does not depend ongio-2.0, so it may fail to compile due to missing Gio include/compile args. Also, since this library is installed, it should carryversion/soversionlikeiptux-coredoes.
gobject_dep = dependency('gobject-2.0')
sigc_dep = dependency('sigc++-2.0')
dependencies = [gobject_dep, sigc_dep]
src/iptux-gi/iptux-service.cpp:63
core_threadis a C++std::shared_ptrmember inside a GObject instance, so it must be explicitly destroyed infinalize(GObject will not run C++ destructors). Also, theconfigproperty should be an object property (not a raw pointer) so it is type-checked and appears correctly in GIR.
obj_properties[PROP_CONFIG] = g_param_spec_pointer(
"config", "Config", "IptuxConfig instance",
static_cast<GParamFlags>(G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY));
g_object_class_install_properties(gobject_class, N_PROPERTIES,
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
src/iptux-gi/iptux-service.cpp:34
- The "config" property is currently treated as an untyped pointer (g_param_spec_pointer + g_value_get_pointer), so IptuxService does not own a reference to the IptuxConfig and can end up with a dangling pointer if the caller unrefs it. For GI-friendly ownership and type safety, make it a GObject property and dup/ref the object here.
switch (property_id) {
case PROP_CONFIG:
self->config = static_cast<::IptuxConfig*>(g_value_get_pointer(value));
break;
src/iptux-gi/iptux-service.cpp:63
- IptuxService's class_init/init currently (1) defines the config property as a raw pointer (so it won’t be introspected as an object type) and (2) initializes core_thread with
0, which is not a valid assignment to std::shared_ptr and can fail compilation under -Werror. Define the property as a GObject (g_param_spec_object), add a dispose handler to unref config, and initialize members with nullptr/reset.
obj_properties[PROP_CONFIG] = g_param_spec_pointer(
"config", "Config", "IptuxConfig instance",
static_cast<GParamFlags>(G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY));
g_object_class_install_properties(gobject_class, N_PROPERTIES,
src/iptux-gi/meson.build:23
- gnome.generate_gir is scanning .cpp sources, but no scanner args are provided to force C++ mode. Without passing
--c++(and explicitly propagating include dirs/deps), g-ir-scanner commonly tries to compile as C and fails on C++ constructs/headers, breaking the GIR build.
gir = gnome.generate_gir(libiptux_gi,
sources: ['iptux-service.h', 'iptux-service.cpp', 'iptux-config.h', 'iptux-config.cpp'],
nsversion: '1.0',
namespace: 'Iptux',
symbol_prefix: 'iptux',
identifier_prefix: 'Iptux',
includes: ['GObject-2.0'],
install: true
Comment on lines
+18
to
+22
| IptuxConfig* iptux_config_new_from_fname(const char* fname) { | ||
| IptuxConfig* self = IPTUX_CONFIG(g_object_new(IPTUX_TYPE_CONFIG, nullptr)); | ||
| self->config = iptux::IptuxConfig::newFromString(fname); | ||
| return self; | ||
| } |
Comment on lines
+11
to
+15
| IptuxService* iptux_service_new(IptuxConfig* config); | ||
|
|
||
| bool iptux_service_start(IptuxService* self); | ||
| bool iptux_service_stop(IptuxService* self); | ||
|
|
Comment on lines
+79
to
+97
| bool iptux_service_start(IptuxService* self) { | ||
| if (!self || !self->core_thread) { | ||
| g_warning("IptuxService or core_thread is null"); | ||
| return false; | ||
| } | ||
|
|
||
| return self->core_thread->start(); | ||
| } | ||
|
|
||
| bool iptux_service_stop(IptuxService* self) { | ||
| if (!self || !self->core_thread) { | ||
| g_warning("IptuxService or core_thread is null"); | ||
| return false; | ||
| } | ||
|
|
||
| self->core_thread->stop(); | ||
| return true; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (6)
src/iptux-gi/iptux-config.cpp:21
iptux_config_new_from_fname()callsIptuxConfig::newFromString(fname), but the API name impliesfnameis a file path. Passing a filename string tonewFromString()will attempt to parse the path as JSON, yielding an invalid/default config and likely breaking CoreThread setup. It also risks UB iffnameis null (conversion tostd::string).
IptuxConfig* iptux_config_new_from_fname(const char* fname) {
IptuxConfig* self = IPTUX_CONFIG(g_object_new(IPTUX_TYPE_CONFIG, nullptr));
self->config = iptux::IptuxConfig::newFromString(fname);
return self;
src/iptux-gi/iptux-service.h:14
- The public C API in this header uses
boolinsideG_BEGIN_DECLS. That makes the header not C-compatible (no<stdbool.h>included) and can break gobject-introspection scanning/bindings. For GObject/GI APIs,gbooleanis the expected return type.
IptuxService* iptux_service_new(IptuxConfig* config);
bool iptux_service_start(IptuxService* self);
bool iptux_service_stop(IptuxService* self);
src/iptux-gi/iptux-service.cpp:96
- Implementation still returns/accepts C++
boolfor the exported C ABI functions. After switching the header togbooleanfor GI/C compatibility, update these implementations to match and returnTRUE/FALSE.
bool iptux_service_start(IptuxService* self) {
if (!self || !self->core_thread) {
g_warning("IptuxService or core_thread is null");
return false;
}
return self->core_thread->start();
}
bool iptux_service_stop(IptuxService* self) {
if (!self || !self->core_thread) {
g_warning("IptuxService or core_thread is null");
return false;
}
self->core_thread->stop();
return true;
}
src/iptux-gi/iptux-service.cpp:29
- The
configconstruct-only property is declared as a generic pointer and retrieved viag_value_get_pointer(). This prevents GI from understanding the property type and makes ownership/lifetime unclear. Prefer anIptuxConfigobject property (g_param_spec_object) and take a reference inset_property, releasing it indispose.
static void iptux_service_set_property(GObject* object,
guint property_id,
const GValue* value,
GParamSpec* pspec) {
IptuxService* self = IPTUX_SERVICE(object);
src/iptux-gi/iptux-service.cpp:60
- After switching the
configproperty to an object, ensure the class installs it asg_param_spec_object(..., IPTUX_TYPE_CONFIG, ...)and wires updisposeso the referenced config is released.
gobject_class->set_property = iptux_service_set_property;
gobject_class->constructed = iptux_service_constructed;
obj_properties[PROP_CONFIG] = g_param_spec_pointer(
"config", "Config", "IptuxConfig instance",
src/iptux-utils/output.cpp:27
- Defaulting the global log level to
DEBUGenables verbose logging for all users by default, which can significantly increase stderr output and impact performance/operability. Unless this is explicitly intended for release builds, it should stay atWARN(or be controlled by a build option/config).
static LogLevel _level = LogLevel::DEBUG;
| browse_url = "" | ||
| repository_url = "" | ||
| website_url = "" | ||
| authors = "Your Name" |
Comment on lines
+74
to
+82
| IptuxService* iptux_service_new(::IptuxConfig* config) { | ||
| return IPTUX_SERVICE( | ||
| g_object_new(IPTUX_TYPE_SERVICE, "config", config, nullptr)); | ||
| } | ||
|
|
||
| bool iptux_service_start(IptuxService* self) { | ||
| if (!self || !self->core_thread) { | ||
| g_warning("IptuxService or core_thread is null"); | ||
| return false; |
lidaobing
marked this pull request as ready for review
July 27, 2026 05:14
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #733 +/- ##
==========================================
+ Coverage 51.62% 52.43% +0.80%
==========================================
Files 64 70 +6
Lines 8867 8994 +127
==========================================
+ Hits 4578 4716 +138
+ Misses 4289 4278 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
❌ The last analysis has failed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce a new GObject-based IptuxService library with introspection support and wire it into the build system.
New Features:
Enhancements: