[PATCH 0/5] MR9075: cfgmgr32: Implement support for logical operators in DEVPROP_FILTER_EXPRESSION.
What about doing this for !9010? I think it'd be simpler. I left the test timeout change aside, as commented on the other MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/aqs.y | 27 ++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dlls/windows.devices.enumeration/aqs.y b/dlls/windows.devices.enumeration/aqs.y index 556900ece17..53ecbd054df 100644 --- a/dlls/windows.devices.enumeration/aqs.y +++ b/dlls/windows.devices.enumeration/aqs.y @@ -90,6 +90,33 @@ expr: #endif (void)yynerrs; /* avoid unused variable warning */ } + | TK_NOT expr expr + { + struct aqs_expr *expr; + + if (FAILED(get_boolean_not_expr( ctx, $2, &expr ))) + YYABORT; + if (FAILED(join_expr( ctx, expr, $3, &$$ ))) + YYABORT; + } + | expr expr TK_OR expr + { + struct aqs_expr *expr; + + if (FAILED(join_expr( ctx, $1, $2, &expr ))) + YYABORT; + if (FAILED(get_boolean_binary_expr( ctx, DEVPROP_OPERATOR_OR_OPEN, expr, $4, &$$ ))) + YYABORT; + } + | expr TK_OR expr expr + { + struct aqs_expr *expr; + + if (FAILED(get_boolean_binary_expr( ctx, DEVPROP_OPERATOR_OR_OPEN, $1, $3, &expr ))) + YYABORT; + if (FAILED(join_expr( ctx, expr, $4, &$$ ))) + YYABORT; + } | expr TK_AND expr { if (FAILED(get_boolean_binary_expr( ctx, DEVPROP_OPERATOR_AND_OPEN, $1, $3, &$$ ))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/cfgmgr32/tests/cfgmgr32.c | 340 +++++++++++++++++++++++++++++++++ 1 file changed, 340 insertions(+) diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index 5841a1a8d4b..d08456521f8 100644 --- a/dlls/cfgmgr32/tests/cfgmgr32.c +++ b/dlls/cfgmgr32/tests/cfgmgr32.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2023 Mohamad Al-Jaf + * Copyright (C) 2025 Vibhav Pant * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -859,6 +860,249 @@ static void test_DevGetObjects( void ) &bool_val, } }; + static const DEVPROP_FILTER_EXPRESSION filter_instance_id_exists = { + DEVPROP_OPERATOR_NOT_EQUALS, { { DEVPKEY_Device_InstanceId, DEVPROP_STORE_SYSTEM, NULL }, DEVPROP_TYPE_EMPTY, 0, NULL } + }; + static const DEVPROP_FILTER_EXPRESSION filter_instance_id_not_exists = { + DEVPROP_OPERATOR_EQUALS, { { DEVPKEY_Device_InstanceId, DEVPROP_STORE_SYSTEM, NULL }, DEVPROP_TYPE_EMPTY, 0, NULL } + }; + /* Test cases for filter expressions containing boolean operators. All of these filters are logically equivalent + * and should return identical objects */ + static const struct { + DEVPROP_FILTER_EXPRESSION expr[12]; + SIZE_T size; + } logical_op_test_cases[] = { + { + /* NOT (System.Devices.InstanceId := []) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 3 + }, + { + /* NOT (NOT (System.Device.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, { DEVPROP_OPERATOR_NOT_OPEN }, + filter_instance_id_exists, + { DEVPROP_OPERATOR_NOT_CLOSE }, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 5 + }, + { + /* NOT ((System.Device.InstanceId := []) AND (System.Device.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_not_exists, filter_instance_id_exists,{ DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE } }, + 6 + }, + { + /* NOT ((NOT (System.Device.InstanceId :- [])) AND (System.Device.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + filter_instance_id_exists, + { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE } }, + 8 + }, + { + /* ((System.Device.InstanceId :- []) OR (System.Device.InstanceId := [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE } }, + 4 + }, + { + /* ((System.Devices.InstanceId :- [] AND System.Devices.InstanceId :- []) OR System.Device.InstanceId := []) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + filter_instance_id_not_exists, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 7 + }, + { + /* ((System.Devices.InstanceId :- [] AND System.Devices.InstanceId :- []) AND System.Device.InstanceId :- []) */ + { { DEVPROP_OPERATOR_AND_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + filter_instance_id_exists, + { DEVPROP_OPERATOR_AND_CLOSE } }, + 7 + }, + { + /* (System.Device.InstanceId :- [] AND (System.Devices.InstanceId :- [] AND System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_AND_OPEN }, + filter_instance_id_exists, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE } }, + 7 + }, + { + /* ((System.Devices.InstanceId := [] OR System.Devices.InstanceId := []) OR System.Device.InstanceId :- []) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE }, + filter_instance_id_exists, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 7 + }, + { + /* (System.Device.InstanceId := [] OR (System.Devices.InstanceId := [] OR System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + filter_instance_id_not_exists, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 7 + }, + /* Logical operators with implicit AND. */ + { + /* (NOT (System.Device.InstanceId := [])) System.Device.InstanceId :- [] */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + filter_instance_id_exists }, + 4 + }, + { + /* (NOT (System.Device.InstanceId := [])) (NOT (System.Device.InstanceId := [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 6 + }, + { + /* (NOT (System.Device.InstanceId :- [] AND System.Device.InstanceId := [])) System.Device.InstanceId :- [] */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE }, + filter_instance_id_exists }, + 7 + }, + { + /* (NOT (System.Device.InstanceId := [] OR System.Device.InstanceId := [])) System.Device.InstanceId :- [] */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE }, + filter_instance_id_exists }, + 7 + }, + { + /* (NOT (System.Device.InstanceId := [] OR System.Device.InstanceId := [])) (System.Device.InstanceId := [] OR System.Device.InstanceId :- []) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_OR_CLOSE } }, + 10 + }, + { + /* (NOT (System.Device.InstanceId := [] OR System.Device.InstanceId := [])) (NOT (System.Device.InstanceId :- [] AND System.Device.InstanceId := [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE } }, + 12 + }, + { + /* ((System.Device.InstanceId := [] AND System.Device.InstanceId := []) OR (Syste.Device.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_not_exists, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE }, + }, + 11 + } + }; + /* Filters that return empty results */ + static const struct { + DEVPROP_FILTER_EXPRESSION expr[14]; + SIZE_T size; + } logical_op_empty_test_cases[] = { + { + /* (NOT (System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 3, + }, + { + /* (NOT (System.Devices.InstanceId :- [])) System.Devices.InstanceId := [] */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + filter_instance_id_not_exists }, + 4, + }, + { + /* (NOT (System.Devices.InstanceId :- [])) (NOT (System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 6, + }, + { + /* (NOT (System.Devices.InstanceId :- [])) (NOT (System.Devices.InstanceId :- [] AND System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE } }, + 9, + }, + { + /* NOT (NOT (System.Devices.InstanceId := [] AND System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_NOT_OPEN }, { DEVPROP_OPERATOR_NOT_OPEN }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_not_exists, filter_instance_id_exists, { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_NOT_CLOSE }, { DEVPROP_OPERATOR_NOT_CLOSE } }, + 8, + }, + { + /* (System.Devices.InstanceId := [] OR System.Devices.InstanceId := []) */ + { { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, filter_instance_id_not_exists, { DEVPROP_OPERATOR_OR_CLOSE } }, + 4, + }, + { + /* (NOT System.Devices.InstanceId :- []) OR (System.Devices.InstanceId := [] OR (NOT System.Devices.InstanceId :- []) OR (NOT System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 14 + }, + { + /* (NOT System.Devices.InstanceId :- []) OR (NOT System.Devices.InstanceId :- []) OR (System.Devices.InstanceId := [] OR (NOT System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 14 + }, + { + /* (NOT System.Devices.InstanceId :- []) OR (NOT System.Devices.InstanceId :- []) OR (System.Devices.InstanceId :- [] AND (NOT System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_OR_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE } }, + 14 + }, + { + /* (NOT System.Devices.InstanceId := []) AND (NOT System.Devices.InstanceId := []) AND (System.Devices.InstanceId :- [] AND (NOT System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_AND_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_AND_OPEN }, filter_instance_id_exists, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE } }, + 14 + }, + { + /* (NOT System.Devices.InstanceId := []) AND (NOT System.Devices.InstanceId := []) AND (System.Devices.InstanceId := [] OR (NOT System.Devices.InstanceId :- [])) */ + { { DEVPROP_OPERATOR_AND_OPEN }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_not_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_OPEN }, filter_instance_id_not_exists, + { DEVPROP_OPERATOR_NOT_OPEN }, filter_instance_id_exists, { DEVPROP_OPERATOR_NOT_CLOSE }, + { DEVPROP_OPERATOR_OR_CLOSE }, + { DEVPROP_OPERATOR_AND_CLOSE } }, + 14 + } + }; DEVPROPCOMPKEY prop_iface_class = { DEVPKEY_DeviceInterface_ClassGuid, DEVPROP_STORE_SYSTEM, NULL }; DEVPROP_FILTER_EXPRESSION filters[4]; const DEV_OBJECT *objects = NULL; @@ -1145,6 +1389,60 @@ static void test_DevGetObjects( void ) } } + memset( filters, 0, sizeof( filters ) ); + for (i = DEVPROP_OPERATOR_GREATER_THAN; i <= DEVPROP_OPERATOR_LESS_THAN_EQUALS; i++) + { + GUID guid = {0}; + + winetest_push_context( "op=%#08lx", i ); + + /* Use the less/greater than operators with GUIDs will never result in a match. */ + filters[0].Operator = i; + filters[0].Property.CompKey.Key = DEVPKEY_DeviceInterface_ClassGuid; + filters[0].Property.Buffer = &guid; + filters[0].Property.BufferSize = sizeof( guid ); + filters[0].Property.Type = DEVPROP_TYPE_GUID; + len = 0xdeadbeef; + objects = (DEV_OBJECT *)0xdeadbeef; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if (len) ok( !len, "got len %lu\n", len ); + todo_wine_if (len) ok( !objects, "got objects %p\n", objects ); + if (objects) pDevFreeObjects( len, objects ); + + /* Consequently, using the DEVPROP_OPERATOR_MODIFIER_NOT modifier will always match. */ + filters[0].Operator |= DEVPROP_OPERATOR_MODIFIER_NOT; + len = 0; + objects = NULL; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if (!len) ok( len > 0, "got len %lu\n", len ); + todo_wine_if (!len) ok( !!objects, "got objects %p\n", objects ); + pDevFreeObjects( len, objects ); + + /* Make sure we get the same results with the max GUID value as well. */ + memset( &guid, 0xff, sizeof( guid ) ); + filters[0].Operator = i; + len = 0xdeadbeef; + objects = (DEV_OBJECT *)0xdeadbeef; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if (len) ok( !len, "got len %lu\n", len ); + todo_wine_if (len) ok( !objects, "got objects %p\n", objects ); + if (objects) pDevFreeObjects( len, objects ); + + filters[0].Operator |= DEVPROP_OPERATOR_MODIFIER_NOT; + len = 0; + objects = NULL; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if (!len) ok( len > 0, "got len %lu\n", len ); + todo_wine_if (!len) ok( !!objects, "got objects %p\n", objects ); + pDevFreeObjects( len, objects ); + + winetest_pop_context(); + } + memset( filters, 0, sizeof( filters ) ); filters[0] = valid_filter; filters[0].Operator = DEVPROP_OPERATOR_NOT_EQUALS; @@ -1158,6 +1456,48 @@ static void test_DevGetObjects( void ) pDevFreeObjects( len, objects ); bool_val = TRUE; + /* Get the number of objects that the filters in logical_op_test_cases should return. */ + filters[0] = filter_instance_id_exists; + len = 0; + objects = NULL; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + ok( len > 0, "got len %lu\n", len ); + ok( !!objects, "got objects %p\n", objects ); + pDevFreeObjects( len, objects ); + + for (i = 0; i < ARRAY_SIZE( logical_op_test_cases ); i++ ) + { + ULONG len2 = 0; + + winetest_push_context( "logical_op_test_cases[%lu]", i ); + + objects = NULL; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, logical_op_test_cases[i].size, + logical_op_test_cases[i].expr, &len2, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if( !len2 ) ok( len2 == len, "got len2 %lu != %lu\n", len2, len ); + todo_wine_if( !len2 ) ok( !!objects, "got objects %p\n", objects ); + pDevFreeObjects( len2, objects ); + + winetest_pop_context(); + } + + for (i = 0; i < ARRAY_SIZE ( logical_op_empty_test_cases ); i++ ) + { + winetest_push_context( "logical_op_empty_test_cases[%lu]", i ); + + len = 0xdeadbeef; + objects = (DEV_OBJECT *)0xdeadbeef; + hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, + logical_op_empty_test_cases[i].size, logical_op_empty_test_cases[i].expr, &len, &objects ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + todo_wine_if( len ) ok( !len, "got len %lu\n", len ); + todo_wine_if( len ) ok( !objects, "got objects %p\n", objects ); + + winetest_pop_context(); + } + for (i = 0; i < ARRAY_SIZE( test_cases ); i++) { const DEV_OBJECT *objects = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/cfgmgr32/main.c | 92 +++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 62 deletions(-) diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c index 69004ea4b5c..b929a6a6e5c 100644 --- a/dlls/cfgmgr32/main.c +++ b/dlls/cfgmgr32/main.c @@ -440,6 +440,20 @@ static HRESULT devprop_filter_eval_compare( const DEV_OBJECT *obj, const DEVPROP return ret ? S_OK : S_FALSE; } +static const DEVPROP_FILTER_EXPRESSION *find_closing_filter( const DEVPROP_FILTER_EXPRESSION *filter, const DEVPROP_FILTER_EXPRESSION *end ) +{ + DWORD open = filter->Operator & DEVPROP_OPERATOR_MASK_LOGICAL, close = open + (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN), depth = 0; + + for (const DEVPROP_FILTER_EXPRESSION *closing = filter + 1; closing < end; closing++) + { + DWORD logical = closing->Operator & DEVPROP_OPERATOR_MASK_LOGICAL; + if (logical == close && !depth--) return closing; + if (logical == open) depth++; + } + + return NULL; +} + /* Return S_OK if the specified filter expressions match the object, S_FALSE if it doesn't. */ static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, ULONG filters_len, const DEVPROP_FILTER_EXPRESSION *filters ) @@ -484,30 +498,6 @@ static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, ULONG filte return hr; } -static HRESULT stack_push( DEVPROP_OPERATOR **stack, ULONG *len, DEVPROP_OPERATOR op ) -{ - DEVPROP_OPERATOR *tmp; - - if (!(tmp = realloc( *stack, (*len + 1) * sizeof( op ) ))) - return E_OUTOFMEMORY; - *stack = tmp; - tmp[*len] = op; - *len += 1; - return S_OK; -} - -static DEVPROP_OPERATOR stack_pop( DEVPROP_OPERATOR **stack, ULONG *len ) -{ - DEVPROP_OPERATOR op = DEVPROP_OPERATOR_NONE; - - if (*len) - { - op = (*stack)[*len - 1]; - *len -= 1; - } - return op; -} - static BOOL devprop_type_validate( DEVPROPTYPE type, ULONG buf_size ) { static const DWORD type_size[] = { @@ -561,15 +551,14 @@ static BOOL devprop_type_validate( DEVPROPTYPE type, ULONG buf_size ) return mod == DEVPROP_TYPEMOD_ARRAY ? buf_size >= size : buf_size == size; } -static HRESULT devprop_filters_validate( ULONG filters_len, const DEVPROP_FILTER_EXPRESSION *filters ) +static BOOL devprop_filters_validate( const DEVPROP_FILTER_EXPRESSION *filters, const DEVPROP_FILTER_EXPRESSION *end ) { - DEVPROP_OPERATOR *stack = NULL; - ULONG i, logical_open = 0, stack_top = 0; - HRESULT hr = S_OK; + const DEVPROP_FILTER_EXPRESSION *closing; - for (i = 0; i < filters_len; i++) + if (filters == end) return FALSE; + + for (const DEVPROP_FILTER_EXPRESSION *filter = filters; filter < end; filter++) { - const DEVPROP_FILTER_EXPRESSION *filter = &filters[i]; const DEVPROPERTY *prop = &filter->Property; DEVPROP_OPERATOR op = filter->Operator; DWORD compare = op & DEVPROP_OPERATOR_MASK_EVAL; @@ -584,16 +573,12 @@ static HRESULT devprop_filters_validate( ULONG filters_len, const DEVPROP_FILTER || !!prop->Buffer != !!prop->BufferSize) { FIXME( "Unknown operator: %#x\n", op ); - hr = E_INVALIDARG; - break; + return FALSE; } if (!op) continue; if (compare && compare != DEVPROP_OPERATOR_EXISTS && !devprop_type_validate( prop->Type, prop->BufferSize )) - { - hr = E_INVALIDARG; - break; - } + return FALSE; switch (modifier) { @@ -602,8 +587,7 @@ static HRESULT devprop_filters_validate( ULONG filters_len, const DEVPROP_FILTER case DEVPROP_OPERATOR_MODIFIER_IGNORE_CASE: break; default: - hr = E_INVALIDARG; - break; + return FALSE; } switch (list) @@ -615,8 +599,7 @@ static HRESULT devprop_filters_validate( ULONG filters_len, const DEVPROP_FILTER case DEVPROP_OPERATOR_LIST_ELEMENT_CONTAINS: break; default: - hr = E_INVALIDARG; - break; + return FALSE; } switch (logical) @@ -626,31 +609,16 @@ static HRESULT devprop_filters_validate( ULONG filters_len, const DEVPROP_FILTER case DEVPROP_OPERATOR_AND_OPEN: case DEVPROP_OPERATOR_OR_OPEN: case DEVPROP_OPERATOR_NOT_OPEN: - hr = stack_push( &stack, &stack_top, logical ); - logical_open = i; + if (!(closing = find_closing_filter( filter, end ))) return FALSE; + if (!devprop_filters_validate( filter + 1, closing )) return FALSE; + filter = closing; break; - case DEVPROP_OPERATOR_AND_CLOSE: - case DEVPROP_OPERATOR_OR_CLOSE: - case DEVPROP_OPERATOR_NOT_CLOSE: - { - DEVPROP_OPERATOR top = stack_pop( &stack, &stack_top ); - /* The operator should be correct paired, and shouldn't be empty. */ - if (logical - top != (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN) || logical_open == i - 1) - hr = E_INVALIDARG; - break; - } default: - hr = E_INVALIDARG; - break; + return FALSE; } - - if (FAILED( hr )) break; } - if (stack_top) - hr = E_INVALIDARG; - free( stack ); - return hr; + return TRUE; } static HRESULT dev_object_iface_get_props( DEV_OBJECT *obj, HDEVINFO set, SP_DEVICE_INTERFACE_DATA *iface_data, @@ -926,7 +894,7 @@ HRESULT WINAPI DevGetObjectsEx( DEV_OBJECT_TYPE type, ULONG flags, ULONG props_l if (!!props_len != !!props || !!filters_len != !!filters || !!params_len != !!params || (flags & ~valid_flags) || (props_len && (flags & DevQueryFlagAllProperties)) - || FAILED( devprop_filters_validate( filters_len, filters ) )) + || (filters && !devprop_filters_validate( filters, filters + filters_len ))) return E_INVALIDARG; if (params) FIXME( "Query parameters are not supported!\n" ); @@ -1378,7 +1346,7 @@ HRESULT WINAPI DevCreateObjectQueryEx( DEV_OBJECT_TYPE type, ULONG flags, ULONG if (!!props_len != !!props || !!filters_len != !!filters || !!params_len != !!params || (flags & ~valid_flags) || !callback || (props_len && (flags & DevQueryFlagAllProperties)) - || FAILED( devprop_filters_validate( filters_len, filters ) )) + || (filters && !devprop_filters_validate( filters, filters + filters_len ))) return E_INVALIDARG; if (filters) FIXME( "Query filters are not supported!\n" ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/cfgmgr32/main.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c index b929a6a6e5c..b7f5051fe47 100644 --- a/dlls/cfgmgr32/main.c +++ b/dlls/cfgmgr32/main.c @@ -455,21 +455,16 @@ static const DEVPROP_FILTER_EXPRESSION *find_closing_filter( const DEVPROP_FILTE } /* Return S_OK if the specified filter expressions match the object, S_FALSE if it doesn't. */ -static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, ULONG filters_len, - const DEVPROP_FILTER_EXPRESSION *filters ) +static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, const DEVPROP_FILTER_EXPRESSION *filters, + const DEVPROP_FILTER_EXPRESSION *end ) { HRESULT hr = S_OK; - ULONG i; - - TRACE( "(%s, %lu, %p)\n", debugstr_DEV_OBJECT( obj ), filters_len, filters ); - if (!filters_len) - return S_OK; + TRACE( "(%s, %p, %p)\n", debugstr_DEV_OBJECT( obj ), filters, end ); /* By default, the evaluation is performed by AND-ing all individual filter expressions. */ - for (i = 0; i < filters_len; i++) + for (const DEVPROP_FILTER_EXPRESSION *filter = filters; filter < end; filter++) { - const DEVPROP_FILTER_EXPRESSION *filter = &filters[i]; DEVPROP_OPERATOR op = filter->Operator; if (op == DEVPROP_OPERATOR_NONE) @@ -747,7 +742,7 @@ static void dev_object_remove_unwanted_props( DEV_OBJECT *obj, ULONG keys_len, c } static HRESULT enum_dev_objects( DEV_OBJECT_TYPE type, ULONG props_len, const DEVPROPCOMPKEY *props, BOOL all_props, - ULONG filters_len, const DEVPROP_FILTER_EXPRESSION *filters, + const DEVPROP_FILTER_EXPRESSION *filters, const DEVPROP_FILTER_EXPRESSION *filters_end, enum_device_object_cb callback, void *data ) { HKEY iface_key; @@ -819,19 +814,16 @@ static HRESULT enum_dev_objects( DEV_OBJECT_TYPE type, ULONG props_len, const DE obj.pszObjectId = detail->DevicePath; /* If we're also filtering objects, get all properties for this object. Once the filters have been * evaluated, free properties that have not been requested, and set cPropertyCount to props_len. */ - if (filters_len) + if (filters) hr = dev_object_iface_get_props( &obj, set, &iface, 0, NULL, TRUE, TRUE ); else hr = dev_object_iface_get_props( &obj, set, &iface, props_len, props, all_props, FALSE ); if (SUCCEEDED( hr )) { - if (filters_len) - { - hr = devprop_filter_matches_object( &obj, filters_len, filters ); - /* Shrink pProperties to only the desired ones, unless DevQueryFlagAllProperties is set. */ - if (!all_props) - dev_object_remove_unwanted_props( &obj, props_len, props ); - } + hr = devprop_filter_matches_object( &obj, filters, filters_end ); + /* Shrink pProperties to only the desired ones, unless DevQueryFlagAllProperties is set. */ + if (!all_props) + dev_object_remove_unwanted_props( &obj, props_len, props ); if (hr == S_OK) hr = callback( obj, data ); else @@ -902,7 +894,7 @@ HRESULT WINAPI DevGetObjectsEx( DEV_OBJECT_TYPE type, ULONG flags, ULONG props_l *objs = NULL; *objs_len = 0; - hr = enum_dev_objects( type, props_len, props, !!(flags & DevQueryFlagAllProperties), filters_len, filters, + hr = enum_dev_objects( type, props_len, props, !!(flags & DevQueryFlagAllProperties), filters, filters + filters_len, dev_objects_append, &objects ); if (SUCCEEDED( hr )) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/cfgmgr32/main.c | 53 ++++++++++++++----- dlls/cfgmgr32/tests/cfgmgr32.c | 24 ++++----- .../tests/devices.c | 4 +- 3 files changed, 54 insertions(+), 27 deletions(-) diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c index b7f5051fe47..8ae08e7c7e8 100644 --- a/dlls/cfgmgr32/main.c +++ b/dlls/cfgmgr32/main.c @@ -17,6 +17,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#include <assert.h> + #include "wine/debug.h" #include "wine/rbtree.h" #include "winreg.h" @@ -414,6 +416,9 @@ static HRESULT devprop_filter_eval_compare( const DEV_OBJECT *obj, const DEVPROP cmp = op & DEVPROP_OPERATOR_MODIFIER_IGNORE_CASE ? wcsicmp( prop->Buffer, cmp_prop->Buffer ) : wcscmp( prop->Buffer, cmp_prop->Buffer ); break; + case DEVPROP_TYPE_GUID: + /* Any other comparison operator other than DEVPROP_OPERATOR_EQUALS with GUIDs evaluates to false. */ + if (!(op & DEVPROP_OPERATOR_EQUALS)) break; default: cmp = memcmp( prop->Buffer, cmp_prop->Buffer, prop->BufferSize ); break; @@ -455,14 +460,13 @@ static const DEVPROP_FILTER_EXPRESSION *find_closing_filter( const DEVPROP_FILTE } /* Return S_OK if the specified filter expressions match the object, S_FALSE if it doesn't. */ -static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, const DEVPROP_FILTER_EXPRESSION *filters, +static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, DEVPROP_OPERATOR op_outer_logical, const DEVPROP_FILTER_EXPRESSION *filters, const DEVPROP_FILTER_EXPRESSION *end ) { HRESULT hr = S_OK; - TRACE( "(%s, %p, %p)\n", debugstr_DEV_OBJECT( obj ), filters, end ); + TRACE( "(%s, %#x, %p, %p)\n", debugstr_DEV_OBJECT( obj ), op_outer_logical, filters, end ); - /* By default, the evaluation is performed by AND-ing all individual filter expressions. */ for (const DEVPROP_FILTER_EXPRESSION *filter = filters; filter < end; filter++) { DEVPROP_OPERATOR op = filter->Operator; @@ -470,26 +474,48 @@ static HRESULT devprop_filter_matches_object( const DEV_OBJECT *obj, const DEVPR if (op == DEVPROP_OPERATOR_NONE) { hr = S_FALSE; - break; } - if (op & (DEVPROP_OPERATOR_MASK_LIST | DEVPROP_OPERATOR_MASK_ARRAY)) + else if (op & (DEVPROP_OPERATOR_MASK_LIST | DEVPROP_OPERATOR_MASK_ARRAY)) { FIXME( "Unsupported list/array operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); - continue; + hr = S_FALSE; } - if (op & DEVPROP_OPERATOR_MASK_LOGICAL) + else if (op & DEVPROP_OPERATOR_MASK_LOGICAL) { - FIXME( "Unsupported logical operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); - continue; + const DEVPROP_FILTER_EXPRESSION *closing = find_closing_filter( filter, end ); + hr = devprop_filter_matches_object( obj, op & DEVPROP_OPERATOR_MASK_LOGICAL, filter + 1, closing ); + filter = closing; } - if (op & DEVPROP_OPERATOR_MASK_EVAL) + else if (op & DEVPROP_OPERATOR_MASK_EVAL) { hr = devprop_filter_eval_compare( obj, filter ); - if (FAILED( hr ) || hr == S_FALSE) - break; + } + if (FAILED( hr )) break; + + /* See if we can short-circuit. */ + switch (op_outer_logical) + { + /* {NOT_OPEN, ..., NOT_CLOSE} is the same as {NOT_OPEN, AND_OPEN, ..., AND_CLOSE, NOT_CLOSE}, so we can + * short circuit here as well. */ + case DEVPROP_OPERATOR_NOT_OPEN: + case DEVPROP_OPERATOR_AND_OPEN: + if (hr == S_FALSE) goto done; + break; + case DEVPROP_OPERATOR_OR_OPEN: + if (hr == S_OK) goto done; + break; + default: + assert( 0 ); + break; } } +done: + if (op_outer_logical == DEVPROP_OPERATOR_NOT_OPEN) + { + if (hr == S_FALSE) hr = S_OK; + else if (hr == S_OK) hr = S_FALSE; + } return hr; } @@ -820,7 +846,8 @@ static HRESULT enum_dev_objects( DEV_OBJECT_TYPE type, ULONG props_len, const DE hr = dev_object_iface_get_props( &obj, set, &iface, props_len, props, all_props, FALSE ); if (SUCCEEDED( hr )) { - hr = devprop_filter_matches_object( &obj, filters, filters_end ); + /* By default, the evaluation is performed by AND-ing all individual filter expressions. */ + hr = devprop_filter_matches_object( &obj, DEVPROP_OPERATOR_AND_OPEN, filters, filters_end ); /* Shrink pProperties to only the desired ones, unless DevQueryFlagAllProperties is set. */ if (!all_props) dev_object_remove_unwanted_props( &obj, props_len, props ); diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index d08456521f8..6e78c2e6aa9 100644 --- a/dlls/cfgmgr32/tests/cfgmgr32.c +++ b/dlls/cfgmgr32/tests/cfgmgr32.c @@ -1406,8 +1406,8 @@ static void test_DevGetObjects( void ) objects = (DEV_OBJECT *)0xdeadbeef; hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if (len) ok( !len, "got len %lu\n", len ); - todo_wine_if (len) ok( !objects, "got objects %p\n", objects ); + ok( !len, "got len %lu\n", len ); + ok( !objects, "got objects %p\n", objects ); if (objects) pDevFreeObjects( len, objects ); /* Consequently, using the DEVPROP_OPERATOR_MODIFIER_NOT modifier will always match. */ @@ -1416,8 +1416,8 @@ static void test_DevGetObjects( void ) objects = NULL; hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if (!len) ok( len > 0, "got len %lu\n", len ); - todo_wine_if (!len) ok( !!objects, "got objects %p\n", objects ); + ok( len > 0, "got len %lu\n", len ); + ok( !!objects, "got objects %p\n", objects ); pDevFreeObjects( len, objects ); /* Make sure we get the same results with the max GUID value as well. */ @@ -1427,8 +1427,8 @@ static void test_DevGetObjects( void ) objects = (DEV_OBJECT *)0xdeadbeef; hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if (len) ok( !len, "got len %lu\n", len ); - todo_wine_if (len) ok( !objects, "got objects %p\n", objects ); + ok( !len, "got len %lu\n", len ); + ok( !objects, "got objects %p\n", objects ); if (objects) pDevFreeObjects( len, objects ); filters[0].Operator |= DEVPROP_OPERATOR_MODIFIER_NOT; @@ -1436,8 +1436,8 @@ static void test_DevGetObjects( void ) objects = NULL; hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, 1, filters, &len, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if (!len) ok( len > 0, "got len %lu\n", len ); - todo_wine_if (!len) ok( !!objects, "got objects %p\n", objects ); + ok( len > 0, "got len %lu\n", len ); + ok( !!objects, "got objects %p\n", objects ); pDevFreeObjects( len, objects ); winetest_pop_context(); @@ -1476,8 +1476,8 @@ static void test_DevGetObjects( void ) hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, logical_op_test_cases[i].size, logical_op_test_cases[i].expr, &len2, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if( !len2 ) ok( len2 == len, "got len2 %lu != %lu\n", len2, len ); - todo_wine_if( !len2 ) ok( !!objects, "got objects %p\n", objects ); + ok( len2 == len, "got len2 %lu != %lu\n", len2, len ); + ok( !!objects, "got objects %p\n", objects ); pDevFreeObjects( len2, objects ); winetest_pop_context(); @@ -1492,8 +1492,8 @@ static void test_DevGetObjects( void ) hr = pDevGetObjects( DevObjectTypeDeviceInterface, DevQueryFlagNone, 0, NULL, logical_op_empty_test_cases[i].size, logical_op_empty_test_cases[i].expr, &len, &objects ); ok( hr == S_OK, "got hr %#lx\n", hr ); - todo_wine_if( len ) ok( !len, "got len %lu\n", len ); - todo_wine_if( len ) ok( !objects, "got objects %p\n", objects ); + ok( !len, "got len %lu\n", len ); + ok( !objects, "got objects %p\n", objects ); winetest_pop_context(); } diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 433f9c33f2b..5c7e1fdc0f2 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -818,7 +818,7 @@ static void test_aqs_filters( void ) test_FindAllAsyncAqsFilter( statics, filters_empty, FALSE, FALSE ); test_CreateWatcherAqsFilter( statics, filters_empty, FALSE, FALSE, FALSE, FALSE ); - test_FindAllAsyncAqsFilter( statics, filters_boolean_op, FALSE, TRUE ); + test_FindAllAsyncAqsFilter( statics, filters_boolean_op, FALSE, FALSE ); test_CreateWatcherAqsFilter( statics, filters_boolean_op, FALSE, FALSE, FALSE, TRUE ); test_FindAllAsyncAqsFilter( statics, filters_simple, FALSE, FALSE ); @@ -827,7 +827,7 @@ static void test_aqs_filters( void ) test_FindAllAsyncAqsFilter( statics, filters_case_insensitive, TRUE, FALSE ); test_CreateWatcherAqsFilter( statics, filters_case_insensitive, TRUE, FALSE, FALSE, FALSE ); - test_FindAllAsyncAqsFilter( statics, filters_precedence, FALSE, TRUE ); + test_FindAllAsyncAqsFilter( statics, filters_precedence, FALSE, FALSE ); test_CreateWatcherAqsFilter( statics, filters_precedence, FALSE, FALSE, FALSE, TRUE ); test_FindAllAsyncAqsFilter( statics, filters_no_results, FALSE, FALSE ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
Yes, this is much cleaner, thanks :) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9075#note_117166
This merge request was approved by Vibhav Pant. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9075
participants (3)
-
Rémi Bernon -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)