Severity: Info
File: src/Servy.Core/Services/ServiceManager.cs, lines 40-80
The file imports using static Servy.Core.Native.NativeMethods; at line 16, then re-declares several Win32 constants that already live in NativeMethods.cs:
// In NativeMethods.cs:
public const int SERVICE_QUERY_CONFIG = 0x0001;
public const int SERVICE_QUERY_STATUS = 0x0004;
public const int SERVICE_WIN32_OWN_PROCESS = 0x00000010;
// In ServiceManager.cs (shadowing — note also int → uint type drift):
public const uint SC_MANAGER_CONNECT = 0x0001;
public const uint SC_MANAGER_CREATE_SERVICE = 0x0002;
public const uint SERVICE_CHANGE_CONFIG = 0x0002;
public const uint SERVICE_QUERY_STATUS = 0x0004; // duplicate of NativeMethods (int)
public const uint SERVICE_START = 0x0010;
public const uint SERVICE_STOP = 0x0020;
public const uint SERVICE_DELETE = 0x00010000;
public const uint SERVICE_WIN32_OWN_PROCESS = 0x00000010; // duplicate of NativeMethods (int)
public const uint SERVICE_ERROR_NORMAL = 0x00000001;
public const int SERVICE_CONFIG_PRESHUTDOWN_INFO = 7;
Two effects:
-
Shadowing — inside ServiceManager, references like SERVICE_QUERY_STATUS resolve to the local uint constant, not the int from NativeMethods. The compiler picks the closer scope. Anyone reading this and assuming "these all come from NativeMethods" can be surprised when an overload picks the uint overload of an API that doesn't exist in NativeMethods's int-based form.
-
Type drift — most P/Invoke signatures in this codebase use int for these masks (see NativeMethods.cs). The uint clones in ServiceManager mean every callsite is doing an implicit int ↔ uint conversion. For values < 0x80000000 it's harmless, but SERVICE_DELETE = 0x00010000 is still positive in int so today nothing breaks. The risk is the next access-right (GENERIC_ALL = 0x10000000) or write-DAC (0x40000) being added with the same mismatch and a future flag bit going negative. This kind of drift is exactly what "single source of truth" prevents.
Suggested fix:
Move the missing constants (SC_MANAGER_*, SERVICE_CHANGE_CONFIG, SERVICE_START, SERVICE_STOP, SERVICE_DELETE, SERVICE_ERROR_NORMAL, SERVICE_CONFIG_PRESHUTDOWN_INFO) into NativeMethods.cs next to their siblings, in a single canonical type (typically int to match existing conventions). Delete the local copies in ServiceManager.cs. The using static Servy.Core.Native.NativeMethods; at line 16 already brings them into scope without qualification.
If access-rights need to be uint because they're passed to OpenService(... uint), change them all to uint once in NativeMethods.cs — picking one type is more important than which type.
Issue #865 covers the same pattern at the struct level (duplicate ServiceStatus/SERVICE_STATUS); this is the constants flavor of the same problem.
Severity: Info
File:
src/Servy.Core/Services/ServiceManager.cs, lines 40-80The file imports
using static Servy.Core.Native.NativeMethods;at line 16, then re-declares several Win32 constants that already live inNativeMethods.cs:Two effects:
Shadowing — inside
ServiceManager, references likeSERVICE_QUERY_STATUSresolve to the localuintconstant, not theintfromNativeMethods. The compiler picks the closer scope. Anyone reading this and assuming "these all come fromNativeMethods" can be surprised when an overload picks theuintoverload of an API that doesn't exist inNativeMethods'sint-based form.Type drift — most P/Invoke signatures in this codebase use
intfor these masks (seeNativeMethods.cs). Theuintclones inServiceManagermean every callsite is doing an implicitint ↔ uintconversion. For values < 0x80000000 it's harmless, butSERVICE_DELETE = 0x00010000is still positive inintso today nothing breaks. The risk is the next access-right (GENERIC_ALL = 0x10000000) or write-DAC (0x40000) being added with the same mismatch and a future flag bit going negative. This kind of drift is exactly what "single source of truth" prevents.Suggested fix:
Move the missing constants (
SC_MANAGER_*,SERVICE_CHANGE_CONFIG,SERVICE_START,SERVICE_STOP,SERVICE_DELETE,SERVICE_ERROR_NORMAL,SERVICE_CONFIG_PRESHUTDOWN_INFO) intoNativeMethods.csnext to their siblings, in a single canonical type (typicallyintto match existing conventions). Delete the local copies inServiceManager.cs. Theusing static Servy.Core.Native.NativeMethods;at line 16 already brings them into scope without qualification.If access-rights need to be
uintbecause they're passed toOpenService(... uint), change them all touintonce inNativeMethods.cs— picking one type is more important than which type.Issue #865 covers the same pattern at the struct level (duplicate
ServiceStatus/SERVICE_STATUS); this is the constants flavor of the same problem.