From 0ad302c11ddae5a8bb6de2c59e908d815f918ab9 Mon Sep 17 00:00:00 2001 From: Simon Rozman Date: Thu, 15 Oct 2020 12:21:55 +0200 Subject: api: stop double error status reporting When an internal function logs an error and its cause, it bloats the log when the caller logs the cause again. Signed-off-by: Simon Rozman --- api/adapter.c | 55 ++++++++++++++++++++++++++----------------------------- api/api.c | 8 ++++---- api/driver.c | 21 ++++++++++++--------- api/logger.h | 6 +++--- api/resource.c | 2 +- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/api/adapter.c b/api/adapter.c index 319e3bc..288bda1 100644 --- a/api/adapter.c +++ b/api/adapter.c @@ -205,7 +205,7 @@ IsOurAdapter(_In_ HDEVINFO DevInfo, _In_ SP_DEVINFO_DATA *DevInfoData, _Out_ BOO WCHAR *Hwids; DWORD Result = GetDeviceRegistryMultiString(DevInfo, DevInfoData, SPDRP_HARDWAREID, &Hwids); if (Result != ERROR_SUCCESS) - return LOG_ERROR(L"Failed to query hardware ID", Result); + return LOG(WINTUN_LOG_ERR, L"Failed to query hardware ID"), Result; *IsOur = DriverIsOurHardwareID(Hwids); return ERROR_SUCCESS; } @@ -293,7 +293,7 @@ ForceCloseWintunAdapterHandle(_In_ HDEVINFO DevInfo, _In_ SP_DEVINFO_DATA *DevIn Result = GetDeviceObject(InstanceId, &NdisHandle); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to get adapter device object", Result); + LOG(WINTUN_LOG_ERR, L"Failed to get adapter device object"); goto out; } Result = DeviceIoControl(NdisHandle, TUN_IOCTL_FORCE_CLOSE_HANDLES, NULL, 0, NULL, 0, &RequiredBytes, NULL) @@ -616,13 +616,13 @@ IsPoolMember( DWORD Result = GetDeviceRegistryString(DevInfo, DevInfoData, SPDRP_DEVICEDESC, &DeviceDesc); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query device description property", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query device description property"); return Result; } Result = GetDeviceRegistryString(DevInfo, DevInfoData, SPDRP_FRIENDLYNAME, &FriendlyName); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query friendly name property", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query friendly name property"); goto cleanupDeviceDesc; } WCHAR PoolDeviceTypeName[MAX_POOL_DEVICE_TYPE]; @@ -706,7 +706,7 @@ CreateAdapterData( Result = RegistryQueryDWORD(Key, L"NetLuidIndex", &(*Adapter)->LuidIndex); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query NetLuidIndex value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query NetLuidIndex value"); goto cleanupAdapter; } @@ -714,7 +714,7 @@ CreateAdapterData( Result = RegistryQueryDWORD(Key, L"*IfType", &(*Adapter)->IfType); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query *IfType value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query *IfType value"); goto cleanupAdapter; } @@ -874,12 +874,12 @@ WintunGetAdapter( Result = IsOurAdapter(DevInfo, &DevInfoData, &IsOur); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to determine hardware ID", Result); + LOG(WINTUN_LOG_ERR, L"Failed to determine hardware ID"); goto cleanupDevInfo; } if (!IsOur) { - LOG_ERROR(L"Foreign adapter with the same name exists", Result); + LOG(WINTUN_LOG_ERR, L"Foreign adapter with the same name exists"); Result = ERROR_ALREADY_EXISTS; goto cleanupDevInfo; } @@ -888,19 +888,19 @@ WintunGetAdapter( Result = IsPoolMember(Pool, DevInfo, &DevInfoData, &IsMember); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to determine pool membership", Result); + LOG(WINTUN_LOG_ERR, L"Failed to determine pool membership"); goto cleanupDevInfo; } if (!IsMember) { - LOG_ERROR(L"Wintun adapter with the same name exists in another pool", Result); + LOG(WINTUN_LOG_ERR, L"Wintun adapter with the same name exists in another pool"); Result = ERROR_ALREADY_EXISTS; goto cleanupDevInfo; } Result = CreateAdapterData(Pool, DevInfo, &DevInfoData, Adapter); if (Result != ERROR_SUCCESS) - LOG_ERROR(L"Failed to create adapter data", Result); + LOG(WINTUN_LOG_ERR, L"Failed to create adapter data"); goto cleanupDevInfo; } @@ -1159,7 +1159,10 @@ WintunCreateAdapter( SP_DRVINFO_DETAIL_DATA_W *DrvInfoDetailData; if (AdapterGetDrvInfoDetail(DevInfo, &DevInfoData, &DrvInfoData, &DrvInfoDetailData) != ERROR_SUCCESS) + { + LOG(WINTUN_LOG_WARN, L"Failed getting driver info detail"); continue; + } if (!DriverIsOurDrvInfoDetail(DrvInfoDetailData)) { HeapFree(Heap, 0, DrvInfoDetailData); @@ -1247,7 +1250,7 @@ WintunCreateAdapter( Result = RegistryQueryStringWait(NetDevRegKey, L"NetCfgInstanceId", WAIT_FOR_REGISTRY_TIMEOUT, &DummyStr); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query NetCfgInstanceId value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query NetCfgInstanceId value"); goto cleanupNetDevRegKey; } HeapFree(Heap, 0, DummyStr); @@ -1255,20 +1258,20 @@ WintunCreateAdapter( Result = RegistryQueryDWORDWait(NetDevRegKey, L"NetLuidIndex", WAIT_FOR_REGISTRY_TIMEOUT, &DummyDWORD); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query NetLuidIndex value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query NetLuidIndex value"); goto cleanupNetDevRegKey; } Result = RegistryQueryDWORDWait(NetDevRegKey, L"*IfType", WAIT_FOR_REGISTRY_TIMEOUT, &DummyDWORD); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query *IfType value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query *IfType value"); goto cleanupNetDevRegKey; } Result = CreateAdapterData(Pool, DevInfo, &DevInfoData, Adapter); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to create adapter data", Result); + LOG(WINTUN_LOG_ERR, L"Failed to create adapter data"); goto cleanupNetDevRegKey; } @@ -1283,13 +1286,13 @@ WintunCreateAdapter( &TcpipAdapterRegKey); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to open adapter-specific TCP/IP adapter registry key", Result); + LOG(WINTUN_LOG_ERR, L"Failed to open adapter-specific TCP/IP adapter registry key"); goto cleanupAdapter; } Result = RegistryQueryStringWait(TcpipAdapterRegKey, L"IpConfig", WAIT_FOR_REGISTRY_TIMEOUT, &DummyStr); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query IpConfig value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query IpConfig value"); goto cleanupTcpipAdapterRegKey; } HeapFree(Heap, 0, DummyStr); @@ -1299,7 +1302,7 @@ WintunCreateAdapter( Result = GetTcpipInterfaceRegPath(*Adapter, TcpipInterfaceRegPath); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to determine interface-specific TCP/IP network registry key path", Result); + LOG(WINTUN_LOG_ERR, L"Failed to determine interface-specific TCP/IP network registry key path"); goto cleanupTcpipAdapterRegKey; } Result = RegistryOpenKeyWait( @@ -1310,7 +1313,7 @@ WintunCreateAdapter( &TcpipInterfaceRegKey); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to open interface-specific TCP/IP network registry key", Result); + LOG(WINTUN_LOG_ERR, L"Failed to open interface-specific TCP/IP network registry key"); goto cleanupTcpipAdapterRegKey; } @@ -1372,7 +1375,7 @@ WintunDeleteAdapter(_In_ const WINTUN_ADAPTER *Adapter, _Inout_ BOOL *RebootRequ return ERROR_SUCCESS; if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to get device info data", Result); + LOG(WINTUN_LOG_ERR, L"Failed to get device info data"); return Result; } SetQuietInstall(DevInfo, &DevInfoData); @@ -1427,20 +1430,14 @@ WintunEnumAdapters(_In_z_count_c_(MAX_POOL) const WCHAR *Pool, _In_ WINTUN_ENUM_ } BOOL IsOur; - Result = IsOurAdapter(DevInfo, &DevInfoData, &IsOur); - if (Result != ERROR_SUCCESS) - { - LOG_ERROR(L"Failed to determine hardware ID", Result); - break; - } - if (!IsOur) + if (IsOurAdapter(DevInfo, &DevInfoData, &IsOur) != ERROR_SUCCESS || !IsOur) continue; BOOL IsMember; Result = IsPoolMember(Pool, DevInfo, &DevInfoData, &IsMember); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to determine pool membership", Result); + LOG(WINTUN_LOG_ERR, L"Failed to determine pool membership"); break; } if (!IsMember) @@ -1450,7 +1447,7 @@ WintunEnumAdapters(_In_z_count_c_(MAX_POOL) const WCHAR *Pool, _In_ WINTUN_ENUM_ Result = CreateAdapterData(Pool, DevInfo, &DevInfoData, &Adapter); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to create adapter data", Result); + LOG(WINTUN_LOG_ERR, L"Failed to create adapter data"); break; } Continue = Func(Adapter, Param); diff --git a/api/api.c b/api/api.c index adb2c13..3b7e895 100644 --- a/api/api.c +++ b/api/api.c @@ -35,24 +35,24 @@ WintunGetVersion( Result = RegistryQueryDWORD(Key, L"DriverMajorVersion", DriverVersionMaj); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query DriverMajorVersion value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query DriverMajorVersion value"); goto cleanupKey; } Result = RegistryQueryDWORD(Key, L"DriverMinorVersion", DriverVersionMin); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query DriverMinorVersion value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query DriverMinorVersion value"); goto cleanupKey; } Result = RegistryQueryDWORD(Key, L"NdisMajorVersion", NdisVersionMaj); if (Result != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to query NdisMajorVersion value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query NdisMajorVersion value"); goto cleanupKey; } Result = RegistryQueryDWORD(Key, L"NdisMinorVersion", NdisVersionMin); if (Result != ERROR_SUCCESS) - LOG_ERROR(L"Failed to query NdisMinorVersion value", Result); + LOG(WINTUN_LOG_ERR, L"Failed to query NdisMinorVersion value"); cleanupKey: RegCloseKey(Key); return Result; diff --git a/api/driver.c b/api/driver.c index 60d9a25..edd144b 100644 --- a/api/driver.c +++ b/api/driver.c @@ -120,7 +120,7 @@ DriverGetVersion(_Out_ FILETIME *DriverDate, _Out_ DWORDLONG *DriverVersion) DWORD SizeResource; DWORD Result = ResourceGetAddress(HaveWHQL() ? L"wintun-whql.inf" : L"wintun.inf", &LockedResource, &SizeResource); if (Result != ERROR_SUCCESS) - return LOG_ERROR(L"Failed to locate resource", Result); + return LOG(WINTUN_LOG_ERR, L"Failed to locate resource"), Result; enum { SectNone, @@ -287,7 +287,7 @@ InstallCertificate(_In_z_ const WCHAR *SignedResource) DWORD SizeResource; DWORD Result = ResourceGetAddress(SignedResource, &LockedResource, &SizeResource); if (Result != ERROR_SUCCESS) - return LOG_ERROR(L"Failed to locate resource", Result); + return LOG(WINTUN_LOG_ERR, L"Failed to locate resource"), Result; const CERT_BLOB CertBlob = { .cbData = SizeResource, .pbData = (BYTE *)LockedResource }; HCERTSTORE QueriedStore; if (!CryptQueryObject( @@ -397,7 +397,7 @@ InstallDriver(_In_ BOOL UpdateExisting) BOOL UseWHQL = HaveWHQL(); if (!UseWHQL && (Result = InstallCertificate(L"wintun.sys")) != ERROR_SUCCESS) - LOG_ERROR(L"Unable to install code signing certificate", Result); + LOG(WINTUN_LOG_WARN, L"Unable to install code signing certificate"); LOG(WINTUN_LOG_INFO, L"Copying resources to temporary path"); if ((Result = ResourceCopyToFile(CatPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.cat" : L"wintun.cat")) != @@ -407,7 +407,7 @@ InstallDriver(_In_ BOOL UpdateExisting) (Result = ResourceCopyToFile(InfPath, &SecurityAttributes, UseWHQL ? L"wintun-whql.inf" : L"wintun.inf")) != ERROR_SUCCESS) { - Result = LOG_LAST_ERROR(L"Failed to copy resources"); + LOG(WINTUN_LOG_ERR, L"Failed to copy resources"); goto cleanupDelete; } @@ -460,7 +460,10 @@ static WINTUN_STATUS RemoveDriver(VOID) } SP_DRVINFO_DETAIL_DATA_W *DrvInfoDetailData; if (AdapterGetDrvInfoDetail(DevInfo, NULL, &DrvInfoData, &DrvInfoDetailData) != ERROR_SUCCESS) + { + LOG(WINTUN_LOG_WARN, L"Failed getting driver info detail"); continue; + } if (!DriverIsOurDrvInfoDetail(DrvInfoDetailData)) { HeapFree(Heap, 0, DrvInfoDetailData); @@ -503,12 +506,12 @@ WINTUN_STATUS DriverInstallOrUpdate(VOID) DWORD Result = ERROR_SUCCESS; if ((Result = RemoveDriver()) != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to uninstall old drivers", Result); + LOG(WINTUN_LOG_ERR, L"Failed to uninstall old drivers"); goto cleanupAdapters; } if ((Result = InstallDriver(!!ExistingAdapters)) != ERROR_SUCCESS) { - LOG_ERROR(L"Failed to install driver", Result); + LOG(WINTUN_LOG_ERR, L"Failed to install driver"); goto cleanupAdapters; } LOG(WINTUN_LOG_INFO, L"Installation successful"); @@ -537,10 +540,10 @@ WINTUN_STATUS DriverUninstall(VOID) { AdapterDeleteAllOurs(); DWORD Result = RemoveDriver(); - if (Result != ERROR_SUCCESS) - LOG_ERROR(L"Failed to uninstall driver", Result); - else + if (Result == ERROR_SUCCESS) LOG(WINTUN_LOG_INFO, L"Uninstallation successful"); + else + LOG(WINTUN_LOG_ERR, L"Failed to uninstall driver"); return Result; } diff --git a/api/logger.h b/api/logger.h index f795a00..e7e3d35 100644 --- a/api/logger.h +++ b/api/logger.h @@ -33,6 +33,6 @@ LoggerLastError(_In_z_ const WCHAR *Prefix) return Error; } -#define LOG(lvl, msg) Logger((lvl), _L(__FUNCTION__) L": " msg) -#define LOG_ERROR(msg, err) LoggerError(_L(__FUNCTION__) L": " msg, (err)) -#define LOG_LAST_ERROR(msg) LoggerLastError(_L(__FUNCTION__) L": " msg) +#define LOG(lvl, msg) (Logger((lvl), _L(__FUNCTION__) L": " msg)) +#define LOG_ERROR(msg, err) (LoggerError(_L(__FUNCTION__) L": " msg, (err))) +#define LOG_LAST_ERROR(msg) (LoggerLastError(_L(__FUNCTION__) L": " msg)) diff --git a/api/resource.c b/api/resource.c index 7b1ea59..e2e5174 100644 --- a/api/resource.c +++ b/api/resource.c @@ -58,7 +58,7 @@ ResourceCopyToFile( DWORD SizeResource; DWORD Result = ResourceGetAddress(ResourceName, &LockedResource, &SizeResource); if (Result != ERROR_SUCCESS) - return LOG_ERROR(L"Failed to locate resource", Result); + return LOG(WINTUN_LOG_ERR, L"Failed to locate resource"), Result; HANDLE DestinationHandle = CreateFileW( DestinationPath, GENERIC_WRITE, -- cgit v1.2.3-59-g8ed1b