-
Notifications
You must be signed in to change notification settings - Fork 386
refactor(log): Replace utility/log with imp/log interface #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
- Replace include "libipc/utility/log.h" with "libipc/imp/log.h" - Replace ipc::error() calls with LIBIPC_LOG() + log.error() - Use type-safe streaming interface instead of printf-style formatting - Remove manual newline characters from log messages
…eue.h - Replace include "libipc/utility/log.h" with "libipc/imp/log.h" - prod_cons.h: Replace ipc::log() calls with LIBIPC_LOG() + log.warning() - Updated 2 force_push() template functions in broadcast implementations - Changed log level from generic log to warning for force_push scenarios - queue.h: Replace ipc::error() calls with LIBIPC_LOG() + log.error() - Updated queue_conn::open() template function - Use type-safe streaming interface instead of printf-style formatting - Remove manual newline characters from log messages
- Update force_push() log calls to use log.debug() instead of log.warning() - Debug level is more appropriate for internal force_push diagnostic messages
- Replace include "libipc/utility/log.h" with "libipc/imp/log.h" - Add LIBIPC_LOG() at the beginning of functions that use logging - Replace all ipc::error() calls with log.error() - Replace all ipc::log() calls with log.debug() or log.error() based on context - Modified functions: - cc_acc(): error logging for shm acquire failure - make_handle(): error logging for chunk storage operations - find_storage(): error logging for invalid storage id - release_storage(): error logging for invalid storage id - recycle_storage(): error logging for invalid storage id - clear_message(): error logging for invalid message size - send(): error logging for various send failures, debug logging for force_push - recv(): error logging for various recv failures - Use type-safe streaming interface instead of printf-style formatting - Remove manual newline characters from log messages - Total changes: 19 log call sites updated
…ync files - Replace include "libipc/utility/log.h" with "libipc/imp/log.h" in all files - Add LIBIPC_LOG() to functions that use logging - Replace ipc::error() and ipc::log() calls with log.error() and log.debug() - Use type-safe streaming interface instead of printf-style formatting - Remove manual newline characters from log messages Modified files: - Linux platform: condition.h, get_wait_time.h, mutex.h, sync_obj_impl.h - POSIX platform: condition.h, get_wait_time.h, mutex.h, semaphore_impl.h, shm_posix.cpp - Windows platform: condition.h, get_sa.h, mutex.h, semaphore.h, shm_win.cpp - Sync layer: condition.cpp, mutex.cpp, semaphore.cpp Total: 17 files updated with comprehensive log interface migration
- Fix multi-parameter log calls with complex formatting in POSIX and Windows platforms - Replace remaining ipc::error() and ipc::log() calls with log.error() and log.warning() - Handle special cases: - POSIX condition.h: pthread_cond_timedwait multi-param formatting - POSIX get_wait_time.h: calc_wait_time multi-param formatting - POSIX semaphore_impl.h: sem_timedwait multi-param formatting - Windows mutex.h: WaitForSingleObject with hex formatting, WAIT_ABANDONED as warning - Windows semaphore.h: WaitForSingleObject and ReleaseSemaphore calls - Use std::hex/std::dec for hexadecimal formatting in Windows platform - All log interface migrations now complete
- Add LIBIPC_LOG() to calc_wait_time() function - Add LIBIPC_LOG() to make_timespec() function - Both functions use log.error() and need the logger initialization
- Add LIBIPC_LOG() to functions in platform files that use log.error/warning/debug - Fixed files: - POSIX platform: mutex.h, semaphore_impl.h, shm_posix.cpp - Windows platform: get_sa.h, mutex.h, semaphore.h, shm_win.cpp - Sync layer: condition.cpp, mutex.cpp, semaphore.cpp All functions using the new log interface now properly initialize the logger with LIBIPC_LOG()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
… files - src/libipc/platform/posix/shm_posix.cpp: * Add LIBIPC_LOG() to acquire() and get_mem() functions * Fix malformed log.error() calls: remove format specifiers (%d, %zd, %p) * Fix parentheses errors in log.error() calls (e.g., .c_str(, "")) * Use stream-based logging instead of printf-style formatting - src/libipc/platform/win/shm_win.cpp: * Add LIBIPC_LOG() to acquire() and get_mem() functions * Fix malformed log.error() calls with GetLastError() * Fix parentheses errors in log.error() calls * Ensure consistent stream-based logging syntax These fixes address syntax errors that would have caused compilation failures.
- src/libipc/platform/posix/condition.h: * Replace all %d and %s format specifiers with stream-based syntax * Update log.error() calls to use proper streaming (e.g., "[", eno, "]") - src/libipc/platform/posix/semaphore_impl.h: * Remove %d format specifiers from log.error() calls * Fix malformed parentheses (e.g., .c_str(, "")) * Remove unnecessary empty string arguments * Use stream-based logging consistently - src/libipc/platform/win/mutex.h: * Fix malformed GetLastError() parentheses * Remove %lu format specifier, use explicit cast instead * Update to stream-based logging syntax - src/libipc/platform/win/semaphore.h: * Fix malformed GetLastError() parentheses * Remove %lu format specifier, use explicit cast instead * Update to stream-based logging syntax All format specifiers (%d, %s, %zd, %p, %lu) have been removed and replaced with proper C++ stream-based logging that is type-safe and consistent with the new imp/log interface.
- src/libipc/prod_cons.h: * Add LIBIPC_LOG() to second force_push() template function * This was missing, causing 'log' to be undeclared at line 379 - src/libipc/ipc.cpp: * Add LIBIPC_LOG() to static send() function (line 590) * Capture log by reference in outer lambda: [tm, &log] * Capture log by reference in inner lambda: [tm, &log, info, que, msg_id] * This fixes 'log' was not declared error in lambda at line 598 * The log variable is now properly captured from the outer send() scope These fixes ensure that all functions using log.debug/error/warning have proper LIBIPC_LOG() initialization and lambda captures.
- src/libipc/ipc.cpp: * Add LIBIPC_LOG() to chunk_storages::get_info() member function * This was missing, causing 'log' to be undeclared at line 245 * The get_info() function uses log.error() for chunk storage errors This completes the fix for all missing LIBIPC_LOG() initializations in the ipc.cpp file.
- src/libipc/platform/win/get_sa.h: * Fix malformed log.error() calls on lines 19 and 23 * Remove extra comma and parenthesis: GetLastError(, -> GetLastError() * Fix closing parenthesis and bracket placement * Line 19: GetLastError(, "]"))) -> GetLastError()), "]" * Line 23: GetLastError(, "]"))) -> GetLastError()), "]" - src/libipc/platform/win/mutex.h: * Add missing LIBIPC_LOG() to try_lock() function at line 84 * The function uses log.error() at line 95 and needs logger initialization These fixes resolve Windows compilation errors related to malformed log calls and missing LIBIPC_LOG() macro.
- src/libipc/platform/win/get_sa.h: * Convert initiator struct to template with Logger parameter * Pass log object from get_sa() to initiator constructor via template * Use 'static initiator<decltype(log)> handle(log)' to instantiate * This allows initiator constructor to properly access log object * Syntax: initiator(Logger const &log) receives the logger - src/libipc/platform/win/semaphore.h: * Fix format error on line 79: remove extra characters '"}]' * Correct closing of log.error() statement * Before: log.error(...)"}] * After: log.error(...); These fixes resolve the static struct initialization issue and code format error in Windows platform.
…iator - src/libipc/platform/win/get_sa.h: * Change from class template to constructor template * Keep 'struct initiator' as a regular class (not template) * Make constructor a function template: template <typename Logger> initiator(Logger const &log) * Instantiate as: static initiator handle(log); * This is valid C++ as function templates can be defined inside functions * Fixes the issue that class templates cannot be defined inside functions The constructor template approach allows proper logger passing while maintaining valid C++ syntax for local struct definitions.
- src/libipc/platform/win/get_sa.h: * Move 'struct initiator' definition outside get_sa() function * Rename to 'sa_initiator' to avoid naming conflicts * Define at namespace ipc::detail scope (above get_sa function) * Keep template constructor: template <typename Logger> sa_initiator(Logger const &log) * get_sa() now simply uses: static sa_initiator handle(log); This fixes the C++ standard violation: - C++03/11/14/17/20 all prohibit local classes from having member templates - Error C2892: 'local class shall not have member templates' - Moving the struct to namespace scope resolves this issue The struct is now a proper namespace-level definition with a template constructor, which is fully compliant with C++ standards.
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.
📋 Overview
This PR begins the migration from the old
src/utility/loginterface to the newinclude/imp/loginterface.🎯 Changes in this PR
File:
src/libipc/shm.cpp#include "libipc/utility/log.h"with#include "libipc/imp/log.h"ipc::error()calls withLIBIPC_LOG()+log.error()🔄 Migration Pattern
Old Interface (utility/log):
New Interface (imp/log):
✨ Benefits
🧪 Testing
Please verify CI compilation passes for all platforms.
📝 Next Steps
After CI verification, will proceed to migrate remaining 19 files.
Related
test/imp/test_imp_log.cpppatternsNote
Replaces legacy utility/log with imp/log across IPC core and platform sync/SHM code, switching to type-safe logging and adding trace hooks, with minor robustness tweaks (mutex/semaphore handling, SHM names).
src/libipc/ipc.cpp,queue.h,prod_cons.h):utility/logwithimp/log; convertipc::error/logcalls tolog.*and addLIBIPC_LOG()tracing.shm.cpp,platform/*/shm_*.cpp):imp/logwith type-safe messages; addLIBIPC_LOG()in key ops (acquire/get_mem/release/remove/sub_ref).imp/log, addLIBIPC_LOG()inopen/lock/wait/notify/broadcast/close/clear.close/clearto avoid robust-list issues; consolidate storage clearing./<name>_sem) and cleanup inclear_storage.sa_initiatorusing logger.sync_obj_impl.h) and time helpers (get_wait_time.h) to new logging API.Written by Cursor Bugbot for commit ab8e6c7. This will update automatically on new commits. Configure here.