Complex event subscription code changes

[ArTitoff]

• Read PR documentation
• Update changelog
• Update documentation

---

Important

Add complex event subscription functionality and modify event emission to support complex events in sc-machine.

• Complex Event Subscription:
  • Added sc_complex_event_subscription_new() and sc_complex_event_subscription_with_user_new() in sc_event_subscription.h and sc_event_subscription.c to create complex event subscriptions.
  • Introduced sc_event_subscription_is_complex() to check if an event subscription is complex.
  • Added sc_event_subscription_add_complex_events_list() and sc_event_subscription_add_one_complex_event() to manage complex event lists.
• Event Emission:
  • Modified _sc_event_emission_manager_add() in sc_event_queue.c to handle complex events by calling start_check_condition_to_activate_complex_event().
  • Updated sc_event_emit_impl() in sc_event_subscription.c to support complex event handling.
• Miscellaneous:
  • Added symbolic links for sc_hash_table.h, sc_monitor_private.h, and sc_mutex_private.h in sc-core/include to their respective locations in sc-store.
  • Commented out extern "C" block in ui.h.

^{This description was created by for ae1ea03. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to ae1ea03 in 3 minutes and 46 seconds. Click for details.

• Reviewed 1354 lines of code in 14 files
• Skipped 2 files when reviewing.
• Skipped posting 8 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. sc-kpm/sc-ui/src/ui.h:10

• Draft comment:
  Verify removal of the extern "C" block around the include. Ensure this change does not break C linkage for C++ builds.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. sc-memory/sc-core/include/sc-container/sc_hash_table.h:1

• Draft comment:
  Avoid embedding a hardcoded relative path. Use proper include directories instead of '../../../src/sc-store/sc-container/sc_hash_table.h'.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a symlink file, not a regular include statement. The comment seems to misunderstand that this is a filesystem symlink, not a C++ include directive. Build systems and include directories are for compile-time includes, while symlinks are filesystem-level constructs. The relative path here is appropriate for a symlink. Maybe there's a good reason to avoid symlinks altogether in the codebase? Maybe there's a better way to structure the includes? While there might be alternative approaches to code organization, symlinks are a valid and common solution, and using relative paths in symlinks is standard practice. The comment's suggestion to use include directories doesn't apply to symlinks. The comment should be deleted because it incorrectly treats a symlink as a C++ include directive and suggests an inapplicable solution.

3. sc-memory/sc-core/include/sc-core/sc_event_subscription.h:72

• Draft comment:
  Add documentation for the new complex event subscription functions and their extra parameters to clarify their purpose and usage.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, asking for documentation to be added. It doesn't suggest a specific change or improvement to the code itself, nor does it address any of the specific rules or guidelines provided.

4. sc-memory/sc-core/src/sc-store/sc_event_subscription.c:172

• Draft comment:
  Revisit the logic for initializing complex event subscriptions. Setting events_list to null for complex subscriptions may be counterintuitive; please clarify or document the design rationale.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 50% <= threshold 50% The comment is asking for clarification or documentation on the design rationale, which is not allowed by the rules. However, it does point out a potential issue with setting events_list to null, which could be a valid concern. The comment could be rephrased to focus on confirming the intention behind setting events_list to null, which is allowed.

5. sc-memory/sc-core/src/sc-store/sc_event_subscription.c:367

• Draft comment:
  Ensure safe handling of the events_list during destruction; check for null before calling sc_list_destroy to prevent potential crashes.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The code already has proper guards in place - events_list is only set to non-null when is_complex_event_subscription is false, and the destroy call is only made when is_complex_event_subscription is true. This means events_list will always be null when the destroy is attempted, making the suggested check redundant. The comment appears to misunderstand the existing control flow. Could there be edge cases I'm missing where events_list could be non-null when is_complex_event_subscription is true? Should I double check the initialization logic? Looking at the initialization in sc_complex_event_subscription_new(), when is_complex_event_subscription is true, events_list is explicitly set to null_ptr. The logic is consistent. The comment should be deleted as it suggests adding redundant null checks when the code already has proper guards through its control flow.

6. sc-memory/sc-core/src/sc-store/sc_event_subscription.c:660

• Draft comment:
  Remove extraneous inline comments (e.g., '// добавил параметр') before committing to production.
• Reason this comment was not posted:
  Marked as duplicate.

7. sc-memory/sc-core/src/sc-store/sc-event/sc_event_private.h:14

• Draft comment:
  There's an extra trailing whitespace at the end of the include directive on line 14. Please remove the unnecessary spaces.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about the whitespace, this is an extremely minor formatting issue. It doesn't affect functionality, readability, or maintainability in any meaningful way. Most IDEs can automatically handle trailing whitespace. This kind of nitpick violates the rule about not making obvious or unimportant comments. The whitespace could theoretically cause issues with some tools or scripts that are sensitive to trailing whitespace. Some coding standards are strict about whitespace. Even if there are tools sensitive to whitespace, this is the kind of issue that should be handled by automated formatting tools or pre-commit hooks, not manual code review comments. This comment should be deleted as it violates the rule about not making obvious or unimportant comments. Whitespace issues should be handled by automated tools.

8. sc-memory/sc-core/src/sc-store/sc-event/sc_event_queue.c:246

• Draft comment:
  Typographical error: The comment contains 'зависисмость'. It should be corrected to 'зависимость'.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% While the comment is technically correct about the typo, it's a very minor issue in a non-English comment. The typo doesn't affect code functionality. The comment is about a changed line, but fixing typos in comments is not a critical code quality issue. However, clear communication is important, and fixing obvious typos helps with code maintenance. The typo is in a comment that appears to be temporary/internal discussion (starts with "Наверное плохо" meaning "Probably bad"), so it may not be worth fixing. The comment itself might be removed later. Even if the comment is temporary, maintaining clear communication standards is valuable, and fixing obvious typos takes minimal effort. Keep the comment but with low confidence, as it's a valid but very minor issue.

Workflow ID: wflow_F59hBEzLsS9y6Krf

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Avoid hardcoding the absolute GLib include path. Replace "/usr/include/glib-2.0/glib.h" with <glib.h> and configure include paths in the build system.

Suggested change

	#include "/usr/include/glib-2.0/glib.h"
	#include <glib.h>

^{This comment was generated because it violated a code review rule: irule_Ah6kBzD0nPDBJTxk.}

[ellipsis-dev]

Review the locking protocol: the code acquires a write lock then later releases it and also calls release_read on the same monitor. Confirm that the read/write lock upgrade/downgrade is correctly handled.

[ellipsis-dev]

The coupling introduced by calling start_check_condition_to_activate_complex_event from within the event manager seems tight. Consider refactoring to separate concerns or clarify dependency.

[ellipsis-dev]

Remove ambiguous inline comments (e.g., '????? type_error with !=') and ensure the comparison logic for events_list is clearly documented.

Suggested change

	if (event_subscription->events_list != null_ptr) //????? type_error with !=
	if (event_subscription->events_list != null_ptr)

[ellipsis-dev]

Review the logic in decrease_thread: the condition and subsequent decrement appear inconsistent. Verify that the intended behavior for decrementing counter_of_activated_events is correctly implemented.

[ellipsis-dev]

Spawning and immediately joining threads in start_check_condition_to_activate_complex_event blocks the caller. Consider redesigning this logic for better asynchronous behavior.