[PATCH v5 0/3] MR9010: cfgmgr32: Device query API part 5: Implement support for logical boolean operators in DEVPROP_FILTER_EXPRESSION.
Additionally, add explicit rules to the AQS parser for resolving shift/reduce conflicts involving NOT/OR clauses. -- v5: cfgmgr32: Implement support for logical operators in DEVPROP_FILTER_EXPRESSION. cfgmgr32/tests: Add tests for filters with boolean operators. https://gitlab.winehq.org/wine/wine/-/merge_requests/9010
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/9010
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/cfgmgr32/tests/cfgmgr32.c | 280 ++++++++++++++++++++++++++++++++- 1 file changed, 279 insertions(+), 1 deletion(-) diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index 5841a1a8d4b..f59805af8f0 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,12 +860,246 @@ 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 { + const char *query; /* The query as an AQS device selector. */ + 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 + } + }; + /* Filters that return empty results */ + static const struct { + const char *query; + 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; DEVPROPCOMPKEY prop_key = {0}; - HRESULT hr; ULONG i, len = 0; + HRESULT hr; if (!pDevGetObjects || !pDevFreeObjects || !pDevFindProperty) { @@ -1158,6 +1393,49 @@ 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] (%s)", i, debugstr_a( logical_op_test_cases[i].query ) ); + + 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] (%s)", i, debugstr_a( logical_op_empty_test_cases[i].query ) ); + + 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 ); + pDevFreeObjects( len, 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/9010
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/cfgmgr32/main.c | 123 +++++++++++++++--- dlls/cfgmgr32/tests/cfgmgr32.c | 4 +- .../tests/devices.c | 4 +- 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c index 69004ea4b5c..ead86cba0e1 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; @@ -440,45 +445,123 @@ static HRESULT devprop_filter_eval_compare( const DEV_OBJECT *obj, const DEVPROP return ret ? S_OK : S_FALSE; } -/* 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 ) +/* Recursively evaluate a sub-expression, returning S_OK on a match or S_FALSE. + * op_outer_logical is the logical operator denoting the outer expression this sub-expression is inside, it should be + * one of the DEVPROP_OPERATOR_*_OPEN operators. + * expr_end_idx gets set to the (relative) index where this sub-expression ends. + */ +static HRESULT devprop_filter_eval_subexpr( const DEV_OBJECT *obj, const DEVPROP_FILTER_EXPRESSION *filters, + const DEVPROP_OPERATOR op_outer_logical, ULONG *expr_end_idx ) { + const DEVPROP_OPERATOR op_outer_close = op_outer_logical + (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN); HRESULT hr = S_OK; ULONG i; - TRACE( "(%s, %lu, %p)\n", debugstr_DEV_OBJECT( obj ), filters_len, filters ); + TRACE( "(%s, %s, %p)\n", debugstr_DEV_OBJECT( obj ), debugstr_DEVPROP_OPERATOR( op_outer_logical ), filters ); - if (!filters_len) - return S_OK; + assert( op_outer_logical == DEVPROP_OPERATOR_AND_OPEN || op_outer_logical == DEVPROP_OPERATOR_OR_OPEN || + op_outer_logical == DEVPROP_OPERATOR_NOT_OPEN ); - /* By default, the evaluation is performed by AND-ing all individual filter expressions. */ - for (i = 0; i < filters_len; i++) + for (i = 0; filters[i].Operator != op_outer_close; i++) { const DEVPROP_FILTER_EXPRESSION *filter = &filters[i]; - DEVPROP_OPERATOR op = filter->Operator; + DEVPROP_OPERATOR op = filter->Operator, logical; - if (op == DEVPROP_OPERATOR_NONE) + logical = op & DEVPROP_OPERATOR_MASK_LOGICAL; + if (logical) { - hr = S_FALSE; - break; + DEVPROP_OPERATOR op_close = logical + (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN); + ULONG last_idx = 0; + + if (FAILED(hr = devprop_filter_eval_subexpr( obj, &filters[i + 1], logical, &last_idx ))) return hr; + i += last_idx + 1; + assert( filters[i].Operator == op_close ); } - if (op & (DEVPROP_OPERATOR_MASK_LIST | DEVPROP_OPERATOR_MASK_ARRAY)) + else if (op == DEVPROP_OPERATOR_NONE) hr = S_FALSE; + else if (op & DEVPROP_OPERATOR_MASK_EVAL) { - FIXME( "Unsupported list/array operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); - continue; + if (FAILED(hr = devprop_filter_eval_compare( obj, filter ))) return hr; } - if (op & DEVPROP_OPERATOR_MASK_LOGICAL) + else { - FIXME( "Unsupported logical operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); + FIXME( "Unsupported operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); continue; } - if (op & DEVPROP_OPERATOR_MASK_EVAL) + + + /* See if we can short-circuit. + * Because this function is recursive and the filter expression is validated, the next CLOSE operator will always + * belong to the current expression, marking its end. */ + switch (op_outer_logical) { - hr = devprop_filter_eval_compare( obj, filter ); - if (FAILED( hr ) || hr == S_FALSE) + /* {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) + { + /* Skip to the end of this sub-expression. */ + while (filters[++i].Operator != op_outer_close) /* nothing */; + goto done; + } + break; + case DEVPROP_OPERATOR_OR_OPEN: + if (hr == S_OK) + { + while (filters[++i].Operator != DEVPROP_OPERATOR_OR_CLOSE) /* nothing */; + goto done; + } break; + DEFAULT_UNREACHABLE; + } + } + +done: + if (SUCCEEDED( hr )) + { + if (expr_end_idx) *expr_end_idx = i; + if (op_outer_logical == DEVPROP_OPERATOR_NOT_OPEN) + hr = (hr == S_OK) ? S_FALSE : S_OK; + } + return hr; +} + +/* 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 ) +{ + HRESULT hr = S_OK; + ULONG i; + + TRACE( "(%s, %lu, %p)\n", debugstr_DEV_OBJECT( obj ), filters_len, filters ); + + if (!filters_len) return S_OK; + + for (i = 0; i < filters_len; i++) + { + const DEVPROP_FILTER_EXPRESSION *filter = &filters[i]; + DEVPROP_OPERATOR op = filter->Operator, logical; + + logical = op & DEVPROP_OPERATOR_MASK_LOGICAL; + if (logical) + { + DEVPROP_OPERATOR op_close = logical + (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN); + ULONG end_idx = 0; + + if (FAILED(hr = devprop_filter_eval_subexpr( obj, &filters[i + 1], logical, &end_idx ))) break; + i += end_idx + 1; + assert( i < filters_len && filters[i].Operator == op_close ); } + else if (op == DEVPROP_OPERATOR_NONE) hr = S_FALSE; + else if (op & DEVPROP_OPERATOR_MASK_EVAL) + hr = devprop_filter_eval_compare( obj, filter ); + else + { + FIXME( "Unsupported operator: %s\n", debugstr_DEVPROP_OPERATOR( op ) ); + continue; + } + + /* By default, the evaluation is performed by AND-ing all individual filter expressions. */ + if (FAILED( hr ) || hr == S_FALSE) break; } return hr; diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index f59805af8f0..e6b4918a100 100644 --- a/dlls/cfgmgr32/tests/cfgmgr32.c +++ b/dlls/cfgmgr32/tests/cfgmgr32.c @@ -1413,8 +1413,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(); 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/9010
On Sat Sep 20 12:56:11 2025 +0000, Vibhav Pant wrote:
changed this file in version 2 of the diff This function is called recursively for every sub-clause, so there would one call frame for the outer `AND_OPEN` expression, and then a recursive call for the inner `AND_OPEN`. Since logical operators are always paired, the code here in the inner call would skip to the nearest AND_CLOSE, set `expr_end_idx`, and then return. The outer call would then continue from `expr_end_idx+1.` I've added some more test cases to demonstrate this.
The function looks complicated, it's hard to tell and there's probably a cleaner way to split the logic.
Yes, this actually is two functions in one, so I have split this into `devprop_filter_eval_subexpr`, which gets called for evaluating sub-expressions inside a `{OPEN, ..., CLOSE}` expression and will recursively evaluate nested expressions, and the original `devprop_filter_matches_object`, which is only used for evaluating the outermost expression. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9010#note_116556
v5: * Move logical subexpression evaluation into `devprop_filter_eval_subexpr`. * Add additional test cases for logical operators. * Move test cases for empty results into `logical_op_empty_test_cases`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9010#note_116557
participants (2)
-
Vibhav Pant -
Vibhav Pant (＠vibhavp)