| Message ID | 20231221073307.2361429-2-gerickson@nuovations.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | service: Improve Logging and Event Visibility at the Info Level | expand |
Hi Grant, > @@ -2195,9 +2195,17 @@ static void cancel_online_check(struct connman_service *service, > static bool online_check_is_enabled_check( > const struct connman_service *service) > { > + g_autofree char *interface = NULL; > + > if (!__connman_service_is_online_check_enabled()) { > - connman_info("Online check disabled. " > - "Default service remains in READY state."); > + interface = connman_service_get_interface(service); > + > + connman_info("Online check disabled; " > + "interface %s [ %s ] remains in %s state.", > + interface, > + __connman_service_type2string(service->type), > + state2string(CONNMAN_SERVICE_STATE_READY)); > + Does it make sense to add a specific method for such informational prints that will enforce the desired formatting pattern instead of relying on human intervention? Regards, -Denis
On Dec 22, 2023, at 10:23 AM, Denis Kenzior <denkenz@gmail.com> wrote: >> @@ -2195,9 +2195,17 @@ static void cancel_online_check(struct connman_service *service, >> static bool online_check_is_enabled_check( >> const struct connman_service *service) >> { >> + g_autofree char *interface = NULL; >> + >> if (!__connman_service_is_online_check_enabled()) { >> - connman_info("Online check disabled. " >> - "Default service remains in READY state."); >> + interface = connman_service_get_interface(service); >> + >> + connman_info("Online check disabled; " >> + "interface %s [ %s ] remains in %s state.", >> + interface, >> + __connman_service_type2string(service->type), >> + state2string(CONNMAN_SERVICE_STATE_READY)); >> + > > Does it make sense to add a specific method for such informational prints that will enforce the desired formatting pattern instead of relying on human intervention? Denis, It’s a great point and one I’d considered. The scope of my changes seemed to small / narrow at that / this juncture. As we review logging across modules and the project at large and there’s an emergent pattern, I’m all for such a function / method. Best, Grant
diff --git a/src/service.c b/src/service.c index 99b322a91684..d3bd1fe83b37 100644 --- a/src/service.c +++ b/src/service.c @@ -2195,9 +2195,17 @@ static void cancel_online_check(struct connman_service *service, static bool online_check_is_enabled_check( const struct connman_service *service) { + g_autofree char *interface = NULL; + if (!__connman_service_is_online_check_enabled()) { - connman_info("Online check disabled. " - "Default service remains in READY state."); + interface = connman_service_get_interface(service); + + connman_info("Online check disabled; " + "interface %s [ %s ] remains in %s state.", + interface, + __connman_service_type2string(service->type), + state2string(CONNMAN_SERVICE_STATE_READY)); + return false; }
This makes the info-level log output of 'online_check_is_enabled_check' consistent with other messages using the format of: ... [Ii]nterface <interface name> [ <service type> ] ... to achieve: Online check disabled; interface wlan0 [ wifi ] remains in ready state. --- src/service.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)