CORE: port ucs logger to ucc

[Sergei-Lebedev]

What

Port UCS logger from libucs to ucc

Why ?

• logger is no longer depend on libucs
• fix traces are not printing
• enable logger features such as file filter, log to file etc.

[greptile-apps]

Greptile Summary

• Ports the UCS logger from libucs to UCC core, creating a self-contained logging system that eliminates dependency on external libucs library
• Adds comprehensive logging configuration options including file filtering, log rotation, buffer sizing, and log-to-file capabilities to enable advanced debugging features
• Updates all existing logging macros across components to use address-of operator (&) for log component parameters, adapting to the new logging interface requirements

Important Files Changed

Filename	Overview
src/utils/debug/log.c	New comprehensive logging implementation with file rotation, filtering, thread-safe operations, but contains potential thread safety issues and uses UCS constants
src/utils/debug/log_def.h	New logging infrastructure header defining core macros and function declarations for the UCC logging system
src/core/ucc_global_opts.c	Adds extensive logging configuration fields and modifies initialization to support new logger features like file output and rotation
src/utils/ucc_sys.c	Adds critical system utility functions for log stream management and process information, but has potential thread safety concerns with static buffers

Confidence score: 3/5

• This PR contains significant architectural changes that require careful review due to potential threading issues and incomplete implementations
• Score lowered due to thread safety concerns in static buffer usage, mixing of UCS constants with UCC code, and placeholder TODO implementations that could cause runtime failures
• Pay close attention to src/utils/debug/log.c for thread safety issues with static buffers and the backtrace function that always aborts execution

[greptile-apps]

Additional Comments (4)

1. src/utils/ucc_assert.h, line 15 (link)

   logic: ucc_assertv_always is referenced but never defined. This will cause a linker error when ENABLE_DEBUG == 1 or UCC_ENABLE_ASSERT == 1.

2. src/utils/ucc_sys.h, line 83-86 (link)

   syntax: ucc_get_exe() is declared twice (also at line 60).

3. src/utils/debug/log_def.h, line 175 (link)

   logic: ucc_component_log_init() is declared but has no implementation in log.c. Either implement it or remove the declaration.

4. src/utils/debug/log_def.h, line 114 (link)

   logic: ucc_log_category_names is declared but never defined anywhere in the codebase.

_{29 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (2)

1. src/utils/ucc_assert.h, line 43-54 (link)

   syntax: Two issues in this macro:

   1. Named ucs_assertv_always but ucc_assertv on line 15 references ucc_assertv_always (undefined)
   2. Uses ucs_likely instead of ucc_likely

2. src/utils/ucc_string.c, line 6-14 (link)

   syntax: Missing #include "ucc_proc_info.h" - ucc_hostname() used on line 306 is declared in that header, not included transitively.

_{29 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (18)

1. src/utils/ucc_proc_info.c, line 282-283 (link)

   logic: redundant assignment - host_hash is set twice consecutively, the first assignment to gethostid() result is immediately overwritten

2. src/utils/ucc_string.h, line 58 (link)

   style: Inconsistent indentation - comment should align with the function declaration below

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

3. src/utils/ucc_string.h, line 109 (link)

   style: Extra leading space before const - should align with other function declarations

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

4. src/utils/ucc_parser.c, line 1002-1013 (link)

   style: Missing newline at end of file

5. src/core/ucc_global_opts.h, line 55-56 (link)

   style: Comment references ucs_print() which suggests this is from the original UCS logger - should this be updated to reflect UCC terminology? Should the comment be updated to use UCC-specific terminology instead of referencing UCS functions?

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

6. src/core/ucc_global_opts.c, line 104 (link)

   style: Reference to 'ucs_print()' in description should be updated to reflect the new UCC logging system. Should this comment reference the new UCC print function instead of ucs_print()?

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

7. src/core/ucc_global_opts.c, line 107 (link)

   syntax: Extra leading space in configuration field definition

8. src/utils/ucc_proc_info.h, line 39 (link)

   logic: The ucc_hostname() function declaration was removed but the function is still implemented in ucc_proc_info.c according to the RAG context. This could break compilation if other files depend on this declaration. Is the ucc_hostname() function declaration being moved to a different header file, or is this function no longer intended to be part of the public API?

9. src/utils/ucc_assert.h, line 34-49 (link)

   style: Inconsistent function name macros: line 34 uses __FUNCTION__ while line 49 uses __func__. Consider using __func__ consistently as it's the C99 standard.

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

10. src/components/tl/ucp/tl_ucp.c, line 214-215 (link)

    logic: Using ucs_offsetof and UCS_CONFIG_TYPE_ON_OFF_AUTO instead of UCC equivalents - this may cause compilation issues after porting the logger from libucs

    Are the UCS types still available after the logger port, or should these be changed to their UCC counterparts?

11. src/utils/debug/log.c, line 439 (link)

    logic: Static buffer is not thread-safe - multiple threads calling this function simultaneously could corrupt the output

12. src/utils/debug/log.c, line 307-308 (link)

    logic: UCS constants used instead of UCC equivalents - should be UCC_KH_PUT_BUCKET_EMPTY and UCC_KH_PUT_BUCKET_CLEAR

    Are these UCS constants intentional or should they be replaced with UCC equivalents?

13. src/utils/ucc_string.c, line 137 (link)

    logic: potential undefined function - ucc_strncpy_safe is used but not defined in this file or visible includes

14. src/utils/ucc_string.c, line 212 (link)

    logic: potential undefined function - ucc_snprintf_safe is used but not defined in this file or visible includes

15. src/utils/ucc_string.c, line 339 (link)

    logic: missing error handling - strdup can return NULL on allocation failure

16. src/utils/debug/log_def.h, line 27 (link)

    style: The cast to ucc_log_component_config_t* assumes the passed pointer is compatible with this type, but there's no compile-time verification of this assumption. Should this cast be replaced with a more type-safe approach or validated at compile time?

    _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

17. src/utils/debug/log_def.h, line 126 (link)

    style: The parameter is not declared as const ucc_log_component_config_t *comp_conf but should be for consistency with the function pointer typedef and to indicate it won't be modified.

    _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

18. src/utils/ucc_sys.c, line 168 (link)

    logic: strndup() can return NULL on allocation failure but result is not checked before passing to ucc_fill_filename_template()

_{32 files reviewed, 18 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (4)

1. src/utils/debug/log.c, line 209 (link)

   style: cast ftell return to size_t before comparison to avoid potential signed/unsigned comparison issues

2. src/utils/ucc_sys.c, line 208 (link)

   logic: check getlogin_r return value - it can fail and return an error code

3. src/utils/ucc_sys.c, line 168-170 (link)

   logic: check strndup return value before using it - allocation can fail

4. src/utils/debug/log.c, line 168-170 (link)

   logic: if allocation fails here, the function continues with uninitialized new_log_file_name, leading to undefined behavior when accessing it in the loop

_{32 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[Sergei-Lebedev]

/build

[Sergei-Lebedev]

/build

[greptile-apps]

Additional Comments (19)

1. src/utils/ucc_sys.h, line 52 (link)

   style: inconsistent indentation - function name should align with other declarations

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

2. src/utils/ucc_sys.h, line 60 (link)

   style: inconsistent indentation - function name should align with other declarations

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

3. src/utils/ucc_sys.h, line 62 (link)

   style: inconsistent indentation - comment should align with other comments

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

4. src/core/ucc_global_opts.c, line 27 (link)

   syntax: Using SIZE_MAX for log_file_size default requires limits.h to be included

5. src/core/ucc_global_opts.c, line 109 (link)

   logic: References ucc_log_level_names which needs to be defined in the included headers. Is ucc_log_level_names defined in one of the newly included debug headers?

6. src/components/tl/ucp/tl_ucp.c, line 214 (link)

   style: Using ucs_offsetof here while other fields use ucc_offsetof - is this intentional? Why does this specific field use ucs_offsetof while all others use ucc_offsetof?

7. src/utils/ucc_assert.h, line 49 (link)

   style: Inconsistent use of __func__ vs __FUNCTION__ - the existing ucc_assert_always uses __FUNCTION__ while the new ucc_assertv_always uses __func__

   Should both macros use the same function name identifier for consistency?

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

8. src/utils/ucc_sys.c, line 207-209 (link)

   logic: getlogin_r can fail but return value is not checked - username buffer may remain uninitialized on failure

9. src/utils/ucc_sys.c, line 217-219 (link)

   logic: gethostname can fail but return value is not checked - hostname buffer may remain uninitialized on failure

10. src/utils/ucc_sys.c, line 224-236 (link)

    syntax: missing #include <sys/syscall.h> and potentially <sys/thr.h> for SYS_gettid and thr_self

11. src/utils/ucc_sys.c, line 215 (link)

    syntax: missing #include <limits.h> for HOST_NAME_MAX constant

12. src/utils/ucc_compiler_def.h, line 49 (link)

    logic: ucc_assert() is called but not declared or included in this header. Should ucc_assert.h be included to provide the ucc_assert() declaration?

13. src/utils/ucc_compiler_def.h, line 47 (link)

    style: leading space before #define should be removed for consistency

    _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

14. src/utils/ucc_compiler_def.h, line 79 (link)

    style: leading space before #define should be removed for consistency

    _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

15. src/utils/debug/log.c, line 307-308 (link)

    logic: uses UCS constants (UCS_KH_PUT_BUCKET_*) instead of UCC equivalents

16. src/utils/debug/log.c, line 439 (link)

    logic: static buffer makes ucc_log_bitmap_to_str non-thread-safe - concurrent calls will corrupt output

17. src/utils/debug/log.c, line 591-592 (link)

    logic: placeholder implementation always aborts - function is unusable

18. src/utils/debug/log_def.h, line 107-110 (link)

    syntax: Parameter message in documentation describes a format string, but the actual parameter name in the function signature should be format for consistency

19. src/utils/ucc_string.c, line 337-341 (link)

    logic: Missing error handling for strdup failure - should check for NULL return and handle memory allocation failure

_{32 files reviewed, 19 comments}

_{Edit Code Review Agent Settings | Greptile}

[Sergei-Lebedev]

/build