Re: [PATCH 5/5] Replace RegisterEventSource with TraceLoggingRegister

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 09/02/2026 15:03, Owen Smith wrote:
>> Could you add some comments to explain the potential issue with
>> RegisterEventSource?
> 
> RegisterEventSource is marked deprecated - CodeQL detected this issue
> https://learn.microsoft.com/en-us/windows-hardware/drivers/devtest/28735-banned-crimson-api-usage
> I will add a comment to this effect.
> 
>> I think it'd be great if you could provide similar patches for
>> xencons_monitor and xenagent as well.
> 
> Similar patches will be created for the other user mode components.
> 
>>> ---
>>> +TRACELOGGING_DEFINE_PROVIDER(g_xenbus_monitor,
>>
>>
>> The provider handle name should use the same convention as other global
>> variables (e.g. MonitorTraceLoggingProvider).
>>
>>> +                             MONITOR_NAME,
>>> +                             //{E8ADEEB7-6DD1-447E-A49B-A6571CC74039}
>>> +                             (0xe8adeeb7, 0x6dd1, 0x447e, 0xa4, 0x9b, 
>>> 0xa6, 0x57, 0x1c, 0xc7, 0x40, 0x39)
>>
>> Is it worth having a DEFINE_GUID for this somewhere?
> 
> This is how the macro is documented - I'd assume the macro will internally 
> define the guid correctly,
> using DEFINE_GUID will cause issues with the trace macro's expansion.
> 

I see, thanks. Was the GUID generated from MONITOR_NAME using the code 
given in TraceLoggingProvider.h or was it randomly generated?

Also, a codestyle nit: the parentheses of function calls are attached to 
the last argument:

     ...,
     MONITOR_NAME,
     //{E8ADEEB7-6DD1-447E-A49B-A6571CC74039}
     (0xe8adeeb7, 0x6dd1, 0x447e, 0xa4, 0x9b, 0xa6, 0x57, 0x1c, 0xc7, 
0x40, 0x39));

(and in the TraceLoggingWrite calls elsewhere)

>>> +    case LOG_INFO:
>>> +        TraceLoggingWrite(g_xenbus_monitor,
>>> +            "Information",
>>> +            TraceLoggingLevel(WINEVENT_LEVEL_INFO),
>>> +            TraceLoggingString(Buffer, "Info")
>>
>> Buffer is a PTSTR, so we should have an appropriate #define for
>> TraceLoggingString.
> 
> 
>>> -        Log("[%u]: %s [%s]",
>>> +        LogInfo("[%u]: %s [%s]",
>>>                SessionId,
>>>                Name,
>>>                WTSStateName(State));
>>
>> Could you align the arguments here (and elsewhere)?
> 
> Sure
> 
> 
> 
> Owen
> 
> 



--
 | Vates

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech