[PATCH v7 0/6] MR8890: Draft: windows.devices.enumeration: Add support for parsing AQS filter strings.
-- v7: maintainers: Add a section for Windows.Devices.Enumeration. windows.devices.enumeration: Create DEVPROP_FILTER_EXPRESSIONs from AQS filter strings and pass them to DevGetObjects. windows.devices.enumeration: Support parsing AQS filters in IDeviceInformationStatics::FindAllAsyncAqsFilter. windows.devices.enumeration/tests: Add tests for IDeviceInformationStatics::FindAllAsyncAqsFilter. propsys: Use VT_LPWSTR as the property type for System.Devices.DeviceInstanceId. https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/cfgmgr32/main.c | 4 +--- dlls/cfgmgr32/tests/cfgmgr32.c | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/dlls/cfgmgr32/main.c b/dlls/cfgmgr32/main.c index 35b78f8f8af..69004ea4b5c 100644 --- a/dlls/cfgmgr32/main.c +++ b/dlls/cfgmgr32/main.c @@ -418,10 +418,8 @@ static HRESULT devprop_filter_eval_compare( const DEV_OBJECT *obj, const DEVPROP cmp = memcmp( prop->Buffer, cmp_prop->Buffer, prop->BufferSize ); break; } - if (op == DEVPROP_OPERATOR_EQUALS) + if (op & DEVPROP_OPERATOR_EQUALS) ret = !cmp; - else if (op & DEVPROP_OPERATOR_EQUALS && !cmp) - ret = TRUE; else ret = (op & DEVPROP_OPERATOR_LESS_THAN) ? cmp < 0 : cmp > 0; } diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c index 2652fac3abb..5841a1a8d4b 100644 --- a/dlls/cfgmgr32/tests/cfgmgr32.c +++ b/dlls/cfgmgr32/tests/cfgmgr32.c @@ -1145,6 +1145,19 @@ static void test_DevGetObjects( void ) } } + memset( filters, 0, sizeof( filters ) ); + filters[0] = valid_filter; + filters[0].Operator = DEVPROP_OPERATOR_NOT_EQUALS; + bool_val = FALSE; + 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 ); + bool_val = TRUE; + for (i = 0; i < ARRAY_SIZE( test_cases ); i++) { const DEV_OBJECT *objects = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/propsys/propsys_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/propsys/propsys_main.c b/dlls/propsys/propsys_main.c index 25d3eea4798..011c05eca96 100644 --- a/dlls/propsys/propsys_main.c +++ b/dlls/propsys/propsys_main.c @@ -829,7 +829,7 @@ static struct system_property_description system_properties[] = { {L"System.Devices.ContainerId", &PKEY_Devices_ContainerId, VT_CLSID}, {L"System.Devices.InterfaceClassGuid", &PKEY_Devices_InterfaceClassGuid, VT_CLSID}, - {L"System.Devices.DeviceInstanceId", &PKEY_Devices_DeviceInstanceId, VT_CLSID}, + {L"System.Devices.DeviceInstanceId", &PKEY_Devices_DeviceInstanceId, VT_LPWSTR}, {L"System.Devices.InterfaceEnabled", &PKEY_Devices_InterfaceEnabled, VT_BOOL}, {L"System.Devices.ClassGuid", &PKEY_Devices_ClassGuid, VT_CLSID}, {L"System.Devices.CompatibleIds", &PKEY_Devices_CompatibleIds, VT_VECTOR | VT_LPWSTR}, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- .../tests/devices.c | 144 +++++++++++++++++- 1 file changed, 141 insertions(+), 3 deletions(-) diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index e4bb1349f6d..840af19e2a1 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -255,9 +255,10 @@ static void await_device_information_collection_( int line, IAsyncOperation_Devi ok_(__FILE__, line)( ret, "CloseHandle failed, error %lu\n", GetLastError() ); } -#define check_device_information_collection_async( a, b, c, d, e ) check_device_information_collection_async_( __LINE__, a, b, c, d, e ) +#define check_device_information_collection_async( a, b, c, d, e ) check_device_information_collection_async_( __LINE__, a, TRUE, b, c, d, e ) +#define check_device_information_collection_async_no_id( a, b, c, d ) check_device_information_collection_async_( __LINE__, a, FALSE, 0, b, c, d ) static void check_device_information_collection_async_( int line, IAsyncOperation_DeviceInformationCollection *async, - UINT32 expect_id, AsyncStatus expect_status, + BOOL test_id, UINT32 expect_id, AsyncStatus expect_status, HRESULT expect_hr, IVectorView_DeviceInformation **result ) { AsyncStatus async_status; @@ -272,7 +273,8 @@ static void check_device_information_collection_async_( int line, IAsyncOperatio hr = IAsyncInfo_get_Id( async_info, &async_id ); if (expect_status < 4) ok_(__FILE__, line)( hr == S_OK, "get_Id returned %#lx\n", hr ); else ok_(__FILE__, line)( hr == E_ILLEGAL_METHOD_CALL, "get_Id returned %#lx\n", hr ); - ok_(__FILE__, line)( async_id == expect_id, "got id %u\n", async_id ); + if (test_id) + ok_(__FILE__, line)( async_id == expect_id, "got id %u\n", async_id ); async_status = 0xdeadbeef; hr = IAsyncInfo_get_Status( async_info, &async_status ); @@ -535,6 +537,141 @@ done: CloseHandle( enumerated_handler.event ); } +struct test_case_filter +{ + const WCHAR *filter; + HRESULT hr; + BOOL no_results; +}; + +#define test_FindAllAsyncAqsFilter( statics, test_cases ) test_FindAllAsyncAqsFilter_( statics, #test_cases, test_cases, ARRAY_SIZE( test_cases ) ) +static void test_FindAllAsyncAqsFilter_( IDeviceInformationStatics *statics, const char *name, const struct test_case_filter *test_cases, SIZE_T len ) +{ + SIZE_T i; + + for (i = 0; i < len; i++) + { + IAsyncOperation_DeviceInformationCollection *devices_async; + IVectorView_DeviceInformation *devices; + UINT32 size; + HSTRING str; + HRESULT hr; + + winetest_push_context("%s[%Iu]", name, i); + hr = WindowsCreateString( test_cases[i].filter, wcslen( test_cases[i].filter ), &str ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + hr = IDeviceInformationStatics_FindAllAsyncAqsFilter( statics, str, &devices_async ); + todo_wine ok( hr == test_cases[i].hr, "gor hr %#lx != %#lx\n", hr, test_cases[i].hr ); + WindowsDeleteString( str ); + if (FAILED( hr ) || FAILED( test_cases[i].hr )) + { + winetest_pop_context(); + continue; + } + await_device_information_collection( devices_async ); + check_device_information_collection_async_no_id( devices_async, Completed, S_OK, &devices ); + + hr = IVectorView_DeviceInformation_get_Size( devices, &size ); + ok( hr == S_OK, "got hr %#lx\n", hr ); + ok( test_cases[i].no_results == !size, "got size %I32u\n", size ); + IAsyncOperation_DeviceInformationCollection_Release( devices_async ); + IVectorView_DeviceInformation_Release( devices ); + + winetest_pop_context(); + } +} + +static const struct test_case_filter simple[] = { + { L"", S_OK }, + { L"System.Devices.InterfaceEnabled := System.StructuredQueryType.Boolean#True", S_OK }, + { L"System.Devices.InterfaceEnabled : System.StructuredQueryType.Boolean#True", S_OK }, + { L"\t\nSystem.Devices.InterfaceEnabled\n\t\n:=\r\n\tSystem.StructuredQueryType.Boolean#True ", S_OK }, + { L"System.Devices.InterfaceEnabled :NOT System.StructuredQueryType.Boolean#False", S_OK }, + { L"System.Devices.InterfaceEnabled :<> System.StructuredQueryType.Boolean#False", S_OK }, + { L"System.Devices.InterfaceEnabled :- System.StructuredQueryType.Boolean#False", S_OK }, + { L"System.Devices.InterfaceEnabled :\u2260 System.StructuredQueryType.Boolean#False", S_OK }, + { L"System.Devices.InterfaceClassGuid :\u2260 {deadbeef-dead-beef-dead-deadbeefdead}", S_OK }, + { L"System.Devices.InterfaceEnabled :NOT []", S_OK }, + { L"System.Devices.InterfaceEnabled := System.StructuredQueryType.Boolean#True " + L"System.Devices.DeviceInstanceId :NOT []", S_OK }, +}; +/* Propsys canonical property names are case-sensitive, but not in AQS. */ +static const struct test_case_filter case_insensitive[] = { + { L"SYSTEM.DEVICES.InterfaceEnabled := System.StructuredQueryType.Boolean#True", S_OK }, + { L"system.devices.interfaceenabled : SYSTEM.STRUCTUREDQUERYTYPE.BOOLEAN#true", S_OK }, + { L"SYSTEM.DEVICES.INTERFACEENABLED :- SYSTEM.STRUCTUREDQUERYTYPE.BOOLEAN#FALSE", S_OK }, + { L"system.devices.interfaceenabled :NOT system.structuredquerytype.boolean#false", S_OK }, + { L"SYSTEM.DEVICES.INTERFACECLASSGUID :\u2260 {DEADBEEF-DEAD-BEEF-DEAD-DEADBEEFDEAD}", S_OK }, +}; +/* Queries that succeed but don't return any results. */ +static const struct test_case_filter no_results[] = { + { L"System.Devices.InterfaceClassGuid := {deadbeef-dead-beef-dead-deadbeefdead}", S_OK, TRUE }, + { L"System.Devices.DeviceInstanceId := \"invalid\\device\\id\"", S_OK, TRUE }, + { L"System.Devices.InterfaceEnabled := []", S_OK, TRUE }, + { L"System.Devices.DeviceInstanceId := []", S_OK, TRUE }, + { L"System.Devices.InterfaceEnabled := System.StructuredQueryType.Boolean#True " + L"System.Devices.InterfaceEnabled := System.StructuredQueryType.Boolean#False", S_OK, TRUE }, + { L"System.Devices.InterfaceClassGuid :< {deadbeef-dead-beef-dead-deadbeefdead} OR " + L"System.Devices.InterfaceClassGuid :> {deadbeef-dead-beef-dead-deadbeefdead}", S_OK, TRUE } +}; +static const struct test_case_filter invalid_comparand_type[] = { + { L"System.Devices.InterfaceEnabled := \"foo\"", E_INVALIDARG }, + { L"System.Devices.InterfaceEnabled := {deadbeef-dead-beef-dead-deadbeefdead}", E_INVALIDARG }, + { L"System.Devices.InterfaceClassGuid := System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfaceClassGuid :- System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfaceEnabled := System.Devices.InterfaceEnabled", E_INVALIDARG }, /* RHS is parsed as a string. */ +}; +static const struct test_case_filter invalid_empty[] = { + { L" ", E_INVALIDARG }, + { L"\t", E_INVALIDARG }, + { L"\n", E_INVALIDARG }, +}; +static const struct test_case_filter invalid_operator[] = { + { L"System.Devices.InterfaceEnabled = System.StructuredQueryType.Boolean#True", E_INVALIDARG }, /* Missing colon */ + { L"System.Devices.InterfacesEnabled :not System.StructuredQueryType.Boolean#True", E_INVALIDARG }, /* Named operators are case-sensitive */ + { L"System.Devices.InterfacesEnabled System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfacesEnabled := := System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfacesEnabled :!= System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfacesEnabled :\U0001F377 System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L"System.Devices.InterfacesEnabled", E_INVALIDARG }, + { L"System.StructuredQueryType.Boolean#True", E_INVALIDARG }, +}; +static const struct test_case_filter invalid_operand[] = { + { L"System.Devices.InterfaceEnabled := ", E_INVALIDARG }, + { L":= System.StructuredQueryType.Boolean#True", E_INVALIDARG }, + { L":=", E_INVALIDARG }, + { L" System.StructuredQueryType.Boolean#True := System.StructuredQueryType.Boolean#True", E_INVALIDARG }, +}; + +static void test_DeviceInformation_filters( void ) +{ + static const WCHAR *class_name = RuntimeClass_Windows_Devices_Enumeration_DeviceInformation; + IDeviceInformationStatics *statics; + HSTRING str; + HRESULT hr; + + hr = WindowsCreateString( class_name, wcslen( class_name ), &str ); + ok(hr == S_OK, "got hr %#lx\n", hr ); + hr = RoGetActivationFactory( str, &IID_IDeviceInformationStatics, (void **)&statics ); + ok( hr == S_OK || broken( hr == REGDB_E_CLASSNOTREG ), "got hr %#lx\n", hr ); + WindowsDeleteString( str ); + if (hr == REGDB_E_CLASSNOTREG) + { + win_skip( "%s runtimeclass, not registered.\n", wine_dbgstr_w( class_name ) ); + return; + } + + test_FindAllAsyncAqsFilter( statics, simple ); + test_FindAllAsyncAqsFilter( statics, case_insensitive ); + test_FindAllAsyncAqsFilter( statics, no_results ); + test_FindAllAsyncAqsFilter( statics, invalid_comparand_type ); + test_FindAllAsyncAqsFilter( statics, invalid_empty ); + test_FindAllAsyncAqsFilter( statics, invalid_operator ); + test_FindAllAsyncAqsFilter( statics, invalid_operand ); + + IDeviceInformationStatics_Release( statics ); +} + static void test_DeviceAccessInformation( void ) { static const WCHAR *device_access_info_name = L"Windows.Devices.Enumeration.DeviceAccessInformation"; @@ -601,6 +738,7 @@ START_TEST( devices ) test_DeviceInformation(); test_DeviceAccessInformation(); + test_DeviceInformation_filters(); RoUninitialize(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/Makefile.in | 2 + dlls/windows.devices.enumeration/aqs.c | 321 ++++++++++++++++++ dlls/windows.devices.enumeration/aqs.h | 115 +++++++ dlls/windows.devices.enumeration/aqs.y | 264 ++++++++++++++ dlls/windows.devices.enumeration/main.c | 14 +- dlls/windows.devices.enumeration/private.h | 1 + .../tests/devices.c | 25 +- 7 files changed, 727 insertions(+), 15 deletions(-) create mode 100644 dlls/windows.devices.enumeration/aqs.c create mode 100644 dlls/windows.devices.enumeration/aqs.h create mode 100644 dlls/windows.devices.enumeration/aqs.y diff --git a/dlls/windows.devices.enumeration/Makefile.in b/dlls/windows.devices.enumeration/Makefile.in index 0a204835ae1..8ca6eb7dedb 100644 --- a/dlls/windows.devices.enumeration/Makefile.in +++ b/dlls/windows.devices.enumeration/Makefile.in @@ -5,6 +5,8 @@ SOURCES = \ access.c \ async.c \ async_private.idl \ + aqs.c \ + aqs.y \ classes.idl \ event_handlers.c \ information.c \ diff --git a/dlls/windows.devices.enumeration/aqs.c b/dlls/windows.devices.enumeration/aqs.c new file mode 100644 index 00000000000..f557e98e25c --- /dev/null +++ b/dlls/windows.devices.enumeration/aqs.c @@ -0,0 +1,321 @@ +/* Advanced Query Syntax parser + * + * Copyright 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> + +#include <windef.h> +#include <devpropdef.h> +#include <devfiltertypes.h> +#include <propvarutil.h> + +#include <wine/debug.h> + +#include "aqs.h" +#include "aqs.tab.h" + +WINE_DEFAULT_DEBUG_CHANNEL( aqs ); + +static const char id_char[] = +{ + 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, + 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1, + 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, +}; + +static int is_idchar( WCHAR chr ) { return chr >= ARRAY_SIZE( id_char ) || id_char[chr]; } + +static int keyword_type( const WCHAR *str, unsigned int len ) +{ + if (!wcsncmp( str, L"AND", len )) + return TK_AND; + if (!wcsncmp( str, L"NOT", len )) + return TK_NOT; + if (!wcsncmp( str, L"OR", len )) + return TK_OR; + if (!wcsnicmp( str, L"System.StructuredQueryType.Boolean#False", len )) + return TK_FALSE; + if (!wcsnicmp( str, L"System.StructuredQueryType.Boolean#True", len )) + return TK_TRUE; + + return TK_ID; +} + +int get_token( const WCHAR *str, aqs_token_kind_t *token ) +{ + int i; + + switch (str[0]) + { + case ' ': + case '\t': + case '\r': + case '\n': + for (i = 1; iswspace( str[i] ); i++); + *token = TK_WHITESPACE; + return i; + case '(': + *token = TK_LEFTPAREN; + return 1; + case ')': + *token = TK_RIGHTPAREN; + return 1; + case '[': + if (str[1] != ']') + { + *token = TK_ILLEGAL; + return 1; + } + *token = TK_NULL; + return 2; + case ':': + *token = TK_COLON; + return 1; + case '=': + *token = TK_EQUAL; + return 1; + case L'\u2260': /* ≠, NOT EQUALS TO */ + *token = TK_NOTEQUAL; + return 1; + case '-': + if (iswdigit(str[1])) + { + *token = TK_MINUS; + return 1; + } + *token = TK_NOTEQUAL; + /* Both -- and - are used as not-equals operators. */ + return str[1] == '-' ? 2 : 1; + case '<': + switch (str[1]) + { + case '=': + *token = TK_LTE; + return 2; + case '>': + *token = TK_NOTEQUAL; + return 2; + default: + *token = TK_LT; + return 1; + } + case L'\u2264': /* ≤, LESS-THAN OR EQUAL TO */ + *token = TK_LTE; + return 1; + case '>': + if (str[1] == '=') + { + *token = TK_GTE; + return 2; + } + *token = TK_GT; + return 1; + case L'\u2265': /* ≥, GREATER-THAN OR EQUAL TO */ + *token = TK_GTE; + return 1; + case '~': + *token = TK_TILDE; + return 1; + case '!': + *token = TK_EXCLAM; + return 1; + case '$': + *token = TK_DOLLAR; + return 1; + case '\"': + for (i = 1; str[i]; i++) + { + if (str[i] == '\"') + { + if (str[i+1] && str[i+1] == '\"') + { + i++; + continue; + } + break; + } + } + if (str[i]) + i++; + *token = TK_STRING; + return i; + case '0': case '1': case '2': case '3': case '4': + case '5': case '6': case '7': case '8': case '9': + *token = TK_INTEGER; + for (i = 1; iswdigit( str[i] ); i++); + return i; + default: + if (!is_idchar( str[0] )) break; + for (i = 1; is_idchar( str[i] ); i++); + *token = keyword_type( str, i ); + return i; + } + *token = TK_ILLEGAL; + return 1; +} + +UINT aqs_lex( void *p, struct aqs_parser *parser ) +{ + aqs_token_kind_t token = -1; + struct string *str = p; + + do + { + parser->idx += parser->len; + if (!parser->query[parser->idx]) return 0; + if (!(parser->len = get_token( &parser->query[parser->idx], &token ))) + break; + str->data = &parser->query[parser->idx]; + str->len = parser->len; + } while (token == TK_WHITESPACE); + return token; +} + +HRESULT aqs_parse_query( const WCHAR *str ) +{ + struct aqs_parser parser = {0}; + HRESULT hr; + int ret; + + if (!wcslen( str )) return S_OK; + + parser.query = str; + if (FAILED(hr = CoCreateInstance( &CLSID_PropertySystem, NULL, CLSCTX_INPROC_SERVER, &IID_IPropertySystem, (void **)&parser.propsys ))) + return hr; + aqs_debug = TRACE_ON( aqs ); + ret = aqs_parse( &parser ); + if (!ret) + FIXME( "semi-stub!\n" ); + else + hr = FAILED(parser.error) ? parser.error : E_INVALIDARG; + + IPropertySystem_Release( parser.propsys ); + return hr; +} + +HRESULT get_integer( struct aqs_parser *parser, PROPVARIANT *val ) +{ + const WCHAR *str = &parser->query[parser->idx]; + int i, num = 0; + + memset( val, 0, sizeof(*val) ); + for (i = 0; i < parser->len; i++) + num = (str[i] - '0') + num * 10; + val->vt = VT_UI4; + val->lVal = num; + return S_OK; +} + +HRESULT get_string( struct aqs_parser *parser, const struct string *p, BOOL id, PROPVARIANT *val ) +{ + const WCHAR *str = p->data; + SIZE_T len = p->len; + WCHAR *buf; + + memset( val, 0, sizeof( *val ) ); + if (!id) + { + str++; + len -= 2; + } + if (!(buf = CoTaskMemAlloc((len + 1) * sizeof( WCHAR )))) + { + parser->error = E_OUTOFMEMORY; + return E_OUTOFMEMORY; + } + memcpy( buf, str, len * sizeof( WCHAR ) ); + buf[len] = 0; + val->vt = VT_LPWSTR; + val->pwszVal = buf; + return S_OK; +} + +HRESULT get_boolean_expr( struct aqs_parser *parser, enum operator_boolean op, struct aqs_expr *lhs, + struct aqs_expr *rhs, struct aqs_expr **ret_expr ) +{ + struct aqs_expr *expr; + + if (!(expr = calloc( 1, sizeof( *expr ) ))) + { + parser->error = E_OUTOFMEMORY; + return E_OUTOFMEMORY; + } + + expr->op_type = OP_TYPE_BOOLEAN; + expr->u.boolean.op = op; + expr->u.boolean.lhs = lhs; + expr->u.boolean.rhs = rhs; + *ret_expr = expr; + return S_OK; +} + +HRESULT get_compare_expr( struct aqs_parser *parser, enum operator_compare op, const WCHAR *prop_name, + const PROPVARIANT *val, struct aqs_expr **ret_expr ) +{ + struct aqs_expr *expr; + HRESULT hr; + + if (!(expr = calloc( 1, sizeof( *expr )))) return (parser->error = E_OUTOFMEMORY); + if (FAILED(hr = IPropertySystem_GetPropertyDescriptionByName( parser->propsys, prop_name, &IID_IPropertyDescription, (void **)&expr->u.compare.prop_desc))) + { + free( expr ); + parser->error = hr == TYPE_E_ELEMENTNOTFOUND ? E_INVALIDARG : hr; + return parser->error; + } + if (FAILED(parser->error = PropVariantCopy( &expr->u.compare.val, val ))) + { + IPropertyDescription_Release( expr->u.compare.prop_desc ); + free( expr ); + return parser->error; + } + + expr->op_type = OP_TYPE_COMPARE; + expr->u.compare.op = op; + parser->expr = expr; + *ret_expr = expr; + return S_OK; +} + +void free_expr( struct aqs_expr *expr ) +{ + switch (expr->op_type) + { + case OP_TYPE_BOOLEAN: + free_expr( expr->u.boolean.lhs ); + free_expr( expr->u.boolean.rhs ); + break; + case OP_TYPE_COMPARE: + IPropertyDescription_Release( expr->u.compare.prop_desc ); + PropVariantClear( &expr->u.compare.val ); + break; + } + free( expr ); +} diff --git a/dlls/windows.devices.enumeration/aqs.h b/dlls/windows.devices.enumeration/aqs.h new file mode 100644 index 00000000000..5f229b2eb6c --- /dev/null +++ b/dlls/windows.devices.enumeration/aqs.h @@ -0,0 +1,115 @@ +/* Advanced Query Syntax parser + * + * Copyright 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <devpropdef.h> +#include <devfiltertypes.h> +#include <wine/list.h> + +#include "private.h" + +struct string +{ + const WCHAR *data; + int len; +}; + +struct aqs_parser +{ + const WCHAR *query; + int idx; + int len; + HRESULT error; + + IPropertySystem *propsys; + struct aqs_expr *expr; +}; + +enum operator_boolean +{ + OP_AND, + OP_OR, + OP_NOT +}; + +enum operator_compare +{ + OP_EQ, + OP_NEQ, + OP_GT, + OP_GTE, + OP_LT, + OP_LTE, + OP_STARTS_WITH, + OP_ENDS_WITH, + OP_CONTAINS, + OP_NOT_CONTAINS, + OP_MATCH_WILDCARD +}; + +enum operator_type +{ + OP_TYPE_BOOLEAN, + OP_TYPE_COMPARE, +}; + +struct aqs_expr; +struct expr_boolean +{ + enum operator_boolean op; + struct aqs_expr *lhs; + struct aqs_expr *rhs; +}; + +enum propval_type +{ + PROPVAL_TYPE_INTEGER, + PROPVAL_TYPE_STRING, + PROPVAL_TYPE_BOOLEAN, + PROPVAL_TYPE_NULL, +}; + +struct expr_compare +{ + enum operator_compare op; + IPropertyDescription *prop_desc; + PROPVARIANT val; +}; + +struct aqs_expr +{ + enum operator_type op_type; + union { + struct expr_boolean boolean; + struct expr_compare compare; + } u; +}; + +extern HRESULT aqs_parse_query( const WCHAR *str ); + +extern UINT aqs_lex( void *val, struct aqs_parser *parser ); + +extern HRESULT get_integer( struct aqs_parser *parser, PROPVARIANT *val ); +extern HRESULT get_string( struct aqs_parser *parser, const struct string *str, BOOL id, PROPVARIANT *val ); + +extern HRESULT get_boolean_expr( struct aqs_parser *parser, enum operator_boolean op, struct aqs_expr *lhs, + struct aqs_expr *rhs, struct aqs_expr **expr ); +extern HRESULT get_compare_expr( struct aqs_parser *parser, enum operator_compare op, const WCHAR *prop_name, + const PROPVARIANT *val, struct aqs_expr **expr ); + +extern void free_expr( struct aqs_expr *expr ); diff --git a/dlls/windows.devices.enumeration/aqs.y b/dlls/windows.devices.enumeration/aqs.y new file mode 100644 index 00000000000..31693e9b8f3 --- /dev/null +++ b/dlls/windows.devices.enumeration/aqs.y @@ -0,0 +1,264 @@ +%{ + +/* Advanced Query Syntax parser + * + * Copyright 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> +#include <stdlib.h> + +#include <windef.h> +#include <winbase.h> + +#include <wine/debug.h> + +#include "aqs.h" + +WINE_DEFAULT_DEBUG_CHANNEL( aqs ); + +#define YYFPRINTF(file, ...) TRACE(__VA_ARGS__) + +static inline const char *debugstr_propvar(const PROPVARIANT *v) +{ + if (!v) + return "(null)"; + + switch (v->vt) + { + case VT_EMPTY: + return wine_dbg_sprintf("%p {VT_EMPTY}", v); + case VT_NULL: + return wine_dbg_sprintf("%p {VT_NULL}", v); + case VT_BOOL: + return wine_dbg_sprintf("%p {VT_BOOL %d}", v, !!v->boolVal); + case VT_UI4: + return wine_dbg_sprintf("%p {VT_UI4: %ld}", v, v->ulVal); + case VT_UI8: + return wine_dbg_sprintf("%p {VT_UI8: %s}", v, wine_dbgstr_longlong(v->uhVal.QuadPart)); + case VT_I8: + return wine_dbg_sprintf("%p {VT_I8: %s}", v, wine_dbgstr_longlong(v->hVal.QuadPart)); + case VT_R4: + return wine_dbg_sprintf("%p {VT_R4: %.8e}", v, v->fltVal); + case VT_R8: + return wine_dbg_sprintf("%p {VT_R8: %lf}", v, v->dblVal); + case VT_CLSID: + return wine_dbg_sprintf("%p {VT_CLSID: %s}", v, wine_dbgstr_guid(v->puuid)); + case VT_LPWSTR: + return wine_dbg_sprintf("%p {VT_LPWSTR: %s}", v, wine_dbgstr_w(v->pwszVal)); + case VT_VECTOR | VT_UI1: + return wine_dbg_sprintf("%p {VT_VECTOR|VT_UI1: %p}", v, v->caub.pElems); + case VT_UNKNOWN: + return wine_dbg_sprintf("%p {VT_UNKNOWN: %p}", v, v->punkVal); + default: + return wine_dbg_sprintf("%p {vt %#x}", v, v->vt); + } +} + +static const char *debugstr_expr( const struct aqs_expr *expr ) +{ + static const char *bool_op[] = { "OP_AND", "OP_OR", "OP_NOT" }; + static const char *compare_op[] = { "OP_EQ", "OP_NEQ", "OP_GT", "OP_GTE", "OP_LT", "OP_LTE", "OP_STARTS_WITH", "OP_ENDS_WITH", "OP_CONTAINS", + "OP_NOT_CONTAINS", "OP_MATCH_WILDCARD" }; + + switch (expr->op_type) + { + case OP_TYPE_BOOLEAN: + if (expr->u.boolean.op < ARRAY_SIZE( bool_op )) + return wine_dbg_sprintf( "{%s %p %p}", bool_op[expr->u.boolean.op], expr->u.boolean.lhs, + expr->u.boolean.rhs ); + return wine_dbg_sprintf( "{(unknown %d) %p %p}", expr->u.boolean.op, expr->u.boolean.lhs, expr->u.boolean.rhs ); + case OP_TYPE_COMPARE: + { + const struct expr_compare *cmp = &expr->u.compare; + if (expr->u.compare.op < ARRAY_SIZE( compare_op )) + return wine_dbg_sprintf( "{%s %p %s}", compare_op[cmp->op], cmp->prop_desc, debugstr_propvar( &cmp->val ) ); + return wine_dbg_sprintf( "{(unknown %d) %p %s}", cmp->op, cmp->prop_desc, debugstr_propvar( &cmp->val ) ); + } + default: + return wine_dbg_sprintf( "{(unknown %d)}", expr->op_type ); + } +} + +static void get_boolean( struct aqs_parser *parser, BOOL b, PROPVARIANT *val ) +{ + memset( val, 0, sizeof( *val ) ); + val->vt = VT_BOOL; + val->boolVal = b ? VARIANT_TRUE : VARIANT_FALSE; +} + +static const PROPVARIANT propval_empty = { VT_EMPTY }; + +static int aqs_error( struct aqs_parser *parser, const char *str ) +{ + if (TRACE_ON( aqs)) ERR( "%s\n", str ); + return 0; +} + +#define GET_COMPARE_EXPR( ctx, op, prop_vt, val_vt, out ) do { \ + HRESULT hr; \ + hr = get_compare_expr( ctx, op, (prop_vt)->pwszVal, val_vt, out ); \ + PropVariantClear( prop_vt ); \ + PropVariantClear( val_vt ); \ + if (FAILED( hr )) \ + YYABORT; \ + } while (0) + +%} + +%lex-param { struct aqs_parser *ctx } +%parse-param { struct aqs_parser *ctx } +%define parse.error verbose +%define api.prefix {aqs_} +%define api.pure + +%union +{ + struct string str; + PROPVARIANT propval; + struct aqs_expr *expr; +} + +%token TK_LEFTPAREN TK_RIGHTPAREN TK_NULL +%token TK_INTEGER TK_WHITESPACE TK_ILLEGAL TK_MINUS +%token TK_TRUE TK_FALSE +%token <str> TK_STRING TK_ID + +%type <propval> id string number boolean null +%type <propval> propval +%type <expr> expr query + +%left TK_AND TK_OR TK_NOT TK_COLON TK_EQUAL TK_NOTEQUAL TK_LT TK_LTE TK_GT TK_GTE TK_TILDE TK_EXCLAM TK_DOLLAR + +%destructor { PropVariantClear( &$$ ); } propval +%destructor { free_expr( $$ ); } expr + +%debug + +%printer { TRACE( "%s", debugstr_wn( $$.data, $$.len ) ); } TK_STRING TK_ID +%printer { TRACE( "%s", debugstr_propvar( &$$ ) ); } id string +%printer { TRACE( "%s", debugstr_propvar( &$$ ) ); } number +%printer { TRACE( "%s", debugstr_propvar( &$$ ) ); } propval +%printer { TRACE( "%s", debugstr_expr( $$ ) ); } expr + +%% + +query: expr { ctx->expr = $1; } + ; +expr: + TK_LEFTPAREN expr TK_RIGHTPAREN + { + $$ = $2; +#if YYBISON >= 30704 + (void)yysymbol_name; /* avoid unused function warning */ +#endif + (void)yynerrs; /* avoid unused variable warning */ + } + | expr TK_AND expr + { + HRESULT hr; + hr = get_boolean_expr( ctx, OP_AND, $1, $3, &$$ ); + if (FAILED( hr )) + YYABORT; + } + | expr TK_OR expr + { + HRESULT hr; + hr = get_boolean_expr( ctx, OP_OR, $1, $3, &$$ ); + if (FAILED( hr )) + YYABORT; + } + | TK_NOT expr + { + HRESULT hr; + hr = get_boolean_expr( ctx, OP_NOT, $2, NULL, &$$ ); + if (FAILED( hr )) + YYABORT; + } + | expr expr + { + HRESULT hr; + hr = get_boolean_expr( ctx, OP_AND, $1, $2, &$$ ); + if (FAILED( hr )) + YYABORT; + } + | id TK_COLON propval { GET_COMPARE_EXPR( ctx, OP_EQ, &$1, &$3, &$$ ); } + | id TK_COLON TK_EQUAL propval { GET_COMPARE_EXPR( ctx, OP_EQ, &$1, &$4, &$$ ); } + | id TK_COLON TK_NOT propval { GET_COMPARE_EXPR( ctx, OP_NEQ, &$1, &$4, &$$ ); } + | id TK_COLON TK_NOTEQUAL propval { GET_COMPARE_EXPR( ctx, OP_NEQ, &$1, &$4, &$$ ); } + | id TK_COLON TK_LT propval { GET_COMPARE_EXPR( ctx, OP_LT, &$1, &$4, &$$ ); } + | id TK_COLON TK_LTE propval { GET_COMPARE_EXPR( ctx, OP_LTE, &$1, &$4, &$$ ); } + | id TK_COLON TK_GT propval { GET_COMPARE_EXPR( ctx, OP_GT, &$1, &$4, &$$ ); } + | id TK_COLON TK_GTE propval { GET_COMPARE_EXPR( ctx, OP_GTE, &$1, &$4, &$$ ); } + /* String operators */ + | id TK_TILDE TK_LT propval { GET_COMPARE_EXPR( ctx, OP_STARTS_WITH, &$1, &$4, &$$ ); } + | id TK_TILDE TK_GT propval { GET_COMPARE_EXPR( ctx, OP_ENDS_WITH, &$1, &$4, &$$ ); } + | id TK_TILDE TK_EQUAL propval { GET_COMPARE_EXPR( ctx, OP_CONTAINS, &$1, &$4, &$$ ); } + | id TK_TILDE TK_TILDE propval { GET_COMPARE_EXPR( ctx, OP_CONTAINS, &$1, &$4, &$$ ); } + | id TK_TILDE TK_EXCLAM propval { GET_COMPARE_EXPR( ctx, OP_NOT_CONTAINS, &$1, &$4, &$$ ); } + | id TK_TILDE propval { GET_COMPARE_EXPR( ctx, OP_MATCH_WILDCARD, &$1, &$3, &$$ ); } + | id TK_DOLLAR TK_EQUAL propval { GET_COMPARE_EXPR( ctx, OP_EQ, &$1, &$4, &$$ ); } + | id TK_DOLLAR TK_DOLLAR propval { GET_COMPARE_EXPR( ctx, OP_EQ, &$1, &$4, &$$ ); } + | id TK_DOLLAR TK_LT propval { GET_COMPARE_EXPR( ctx, OP_STARTS_WITH, &$1, &$4, &$$ ); } +propval: + id | string | number | boolean | null + ; +id: + TK_ID + { + HRESULT hr; + hr = get_string( ctx, &$1, TRUE, &$$ ); + if (FAILED( hr )) + YYABORT; + } + ; +string: + TK_STRING + { + HRESULT hr; + hr = get_string( ctx, &$1, FALSE, &$$ ); + if (FAILED( hr )) + YYABORT; + } + ; +number: + TK_INTEGER + { + HRESULT hr; + hr = get_integer( ctx, &$$ ); + if (FAILED( hr )) + YYABORT; + } + ; +boolean: + TK_TRUE + { + get_boolean( ctx, TRUE, &$$ ); + } + ; + | TK_FALSE + { + get_boolean( ctx, FALSE, &$$ ); + } + ; +null: + TK_NULL + { + $$ = propval_empty; + } + ; +%% diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index f8462c8faed..ca5af3f2100 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -19,6 +19,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#define COBJMACROS #include <assert.h> #include "initguid.h" @@ -26,6 +27,7 @@ #include "devpropdef.h" #include "devfiltertypes.h" #include "devquery.h" +#include "aqs.h" #include "wine/debug.h" @@ -575,8 +577,7 @@ static HRESULT WINAPI device_statics_FindAllAsync( IDeviceInformationStatics *if IAsyncOperation_DeviceInformationCollection **op ) { TRACE( "iface %p, op %p\n", iface, op ); - return async_operation_inspectable_create( &IID_IAsyncOperation_DeviceInformationCollection, (IUnknown *)iface, NULL, - find_all_async, (IAsyncOperation_IInspectable **)op ); + return IDeviceInformationStatics_FindAllAsyncAqsFilter( iface, NULL, op ); } static HRESULT WINAPI device_statics_FindAllAsyncDeviceClass( IDeviceInformationStatics *iface, DeviceClass class, @@ -589,8 +590,13 @@ static HRESULT WINAPI device_statics_FindAllAsyncDeviceClass( IDeviceInformation static HRESULT WINAPI device_statics_FindAllAsyncAqsFilter( IDeviceInformationStatics *iface, HSTRING filter, IAsyncOperation_DeviceInformationCollection **op ) { - FIXME( "iface %p, aqs %p, op %p stub!\n", iface, debugstr_hstring(filter), op ); - return E_NOTIMPL; + HRESULT hr; + + TRACE( "iface %p, aqs %p, op %p\n", iface, debugstr_hstring(filter), op ); + + if (FAILED(hr = aqs_parse_query(WindowsGetStringRawBuffer( filter, NULL )))) return hr; + return async_operation_inspectable_create( &IID_IAsyncOperation_DeviceInformationCollection, (IUnknown *)iface, NULL, + find_all_async, (IAsyncOperation_IInspectable **)op ); } static HRESULT WINAPI device_statics_FindAllAsyncAqsFilterAndAdditionalProperties( IDeviceInformationStatics *iface, HSTRING filter, diff --git a/dlls/windows.devices.enumeration/private.h b/dlls/windows.devices.enumeration/private.h index ad15b7916ca..0ce5624b21c 100644 --- a/dlls/windows.devices.enumeration/private.h +++ b/dlls/windows.devices.enumeration/private.h @@ -28,6 +28,7 @@ #include "winbase.h" #include "winstring.h" #include "objbase.h" +#include "propsys.h" #include "activation.h" diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 840af19e2a1..8fbfd9b50af 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -544,8 +544,11 @@ struct test_case_filter BOOL no_results; }; -#define test_FindAllAsyncAqsFilter( statics, test_cases ) test_FindAllAsyncAqsFilter_( statics, #test_cases, test_cases, ARRAY_SIZE( test_cases ) ) -static void test_FindAllAsyncAqsFilter_( IDeviceInformationStatics *statics, const char *name, const struct test_case_filter *test_cases, SIZE_T len ) +#define test_FindAllAsyncAqsFilter( statics, test_cases, todo_hr, todo_results ) \ + test_FindAllAsyncAqsFilter_( statics, #test_cases, test_cases, ARRAY_SIZE( test_cases ), todo_hr, todo_results ) + +static void test_FindAllAsyncAqsFilter_( IDeviceInformationStatics *statics, const char *name, const struct test_case_filter *test_cases, SIZE_T len, + BOOL todo_hr, BOOL todo_results ) { SIZE_T i; @@ -561,7 +564,7 @@ static void test_FindAllAsyncAqsFilter_( IDeviceInformationStatics *statics, con hr = WindowsCreateString( test_cases[i].filter, wcslen( test_cases[i].filter ), &str ); ok( hr == S_OK, "got hr %#lx\n", hr ); hr = IDeviceInformationStatics_FindAllAsyncAqsFilter( statics, str, &devices_async ); - todo_wine ok( hr == test_cases[i].hr, "gor hr %#lx != %#lx\n", hr, test_cases[i].hr ); + todo_wine_if( todo_hr ) ok( hr == test_cases[i].hr, "gor hr %#lx != %#lx\n", hr, test_cases[i].hr ); WindowsDeleteString( str ); if (FAILED( hr ) || FAILED( test_cases[i].hr )) { @@ -573,7 +576,7 @@ static void test_FindAllAsyncAqsFilter_( IDeviceInformationStatics *statics, con hr = IVectorView_DeviceInformation_get_Size( devices, &size ); ok( hr == S_OK, "got hr %#lx\n", hr ); - ok( test_cases[i].no_results == !size, "got size %I32u\n", size ); + todo_wine_if( todo_results ) ok( test_cases[i].no_results == !size, "got size %I32u\n", size ); IAsyncOperation_DeviceInformationCollection_Release( devices_async ); IVectorView_DeviceInformation_Release( devices ); @@ -661,13 +664,13 @@ static void test_DeviceInformation_filters( void ) return; } - test_FindAllAsyncAqsFilter( statics, simple ); - test_FindAllAsyncAqsFilter( statics, case_insensitive ); - test_FindAllAsyncAqsFilter( statics, no_results ); - test_FindAllAsyncAqsFilter( statics, invalid_comparand_type ); - test_FindAllAsyncAqsFilter( statics, invalid_empty ); - test_FindAllAsyncAqsFilter( statics, invalid_operator ); - test_FindAllAsyncAqsFilter( statics, invalid_operand ); + test_FindAllAsyncAqsFilter( statics, simple, FALSE, FALSE ); + test_FindAllAsyncAqsFilter( statics, case_insensitive, TRUE, FALSE ); + test_FindAllAsyncAqsFilter( statics, no_results, FALSE, TRUE ); + test_FindAllAsyncAqsFilter( statics, invalid_comparand_type, TRUE, FALSE ); + test_FindAllAsyncAqsFilter( statics, invalid_empty, FALSE, FALSE ); + test_FindAllAsyncAqsFilter( statics, invalid_operator, FALSE, FALSE ); + test_FindAllAsyncAqsFilter( statics, invalid_operand, FALSE, FALSE ); IDeviceInformationStatics_Release( statics ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/windows.devices.enumeration/Makefile.in | 2 +- dlls/windows.devices.enumeration/aqs.c | 210 +++++++++++++++++- dlls/windows.devices.enumeration/aqs.h | 3 +- dlls/windows.devices.enumeration/main.c | 91 +++++++- .../tests/devices.c | 4 +- 5 files changed, 300 insertions(+), 10 deletions(-) diff --git a/dlls/windows.devices.enumeration/Makefile.in b/dlls/windows.devices.enumeration/Makefile.in index 8ca6eb7dedb..525c55d353b 100644 --- a/dlls/windows.devices.enumeration/Makefile.in +++ b/dlls/windows.devices.enumeration/Makefile.in @@ -1,5 +1,5 @@ MODULE = windows.devices.enumeration.dll -IMPORTS = cfgmgr32 combase uuid +IMPORTS = cfgmgr32 combase propsys uuid SOURCES = \ access.c \ diff --git a/dlls/windows.devices.enumeration/aqs.c b/dlls/windows.devices.enumeration/aqs.c index f557e98e25c..e8bcff07ef6 100644 --- a/dlls/windows.devices.enumeration/aqs.c +++ b/dlls/windows.devices.enumeration/aqs.c @@ -199,12 +199,16 @@ UINT aqs_lex( void *p, struct aqs_parser *parser ) return token; } -HRESULT aqs_parse_query( const WCHAR *str ) +static HRESULT aqs_expr_to_filters( const struct aqs_expr *expr, DEVPROP_FILTER_EXPRESSION **filters, ULONG *filters_len ); + +HRESULT aqs_parse_query( const WCHAR *str, DEVPROP_FILTER_EXPRESSION **filters, ULONG *filters_len ) { struct aqs_parser parser = {0}; HRESULT hr; int ret; + *filters = NULL; + *filters_len = 0; if (!wcslen( str )) return S_OK; parser.query = str; @@ -212,8 +216,11 @@ HRESULT aqs_parse_query( const WCHAR *str ) return hr; aqs_debug = TRACE_ON( aqs ); ret = aqs_parse( &parser ); - if (!ret) - FIXME( "semi-stub!\n" ); + if (!ret && parser.expr) + { + hr = aqs_expr_to_filters( parser.expr, filters, filters_len ); + free_expr( parser.expr ); + } else hr = FAILED(parser.error) ? parser.error : E_INVALIDARG; @@ -319,3 +326,200 @@ void free_expr( struct aqs_expr *expr ) } free( expr ); } + +static HRESULT propval_to_devprop( const PROPVARIANT *comparand_val, IPropertyDescription *desc, DEVPROPERTY *devprop ) +{ + union + { + BYTE byte; + UINT16 int16; + UINT32 int32; + UINT64 int64; + GUID guid; + DEVPROP_BOOLEAN boolean; + } devprop_basic_val = {0}; + PROPVARIANT tmp = {0}; + VARTYPE prop_vt; + HRESULT hr; + + if(FAILED(hr = IPropertyDescription_GetPropertyKey( desc, (PROPERTYKEY *)&devprop->CompKey.Key))) return hr; + if (FAILED(hr = IPropertyDescription_GetPropertyType( desc, &prop_vt ))) return hr; + if (comparand_val->vt != VT_EMPTY) + { + if (FAILED(hr = PropVariantChangeType( &tmp, comparand_val, 0, prop_vt ))) + return (hr == E_FAIL || hr == E_NOTIMPL) ? E_INVALIDARG : hr; + switch (prop_vt) + { + case VT_CLSID: + devprop->Type = DEVPROP_TYPE_GUID; + devprop_basic_val.guid = *tmp.puuid; + devprop->BufferSize = sizeof( devprop_basic_val.guid ); + break; + case VT_I1: + case VT_UI1: + devprop->Type = prop_vt == VT_I1 ? DEVPROP_TYPE_SBYTE : DEVPROP_TYPE_BYTE; + devprop_basic_val.byte = tmp.bVal; + devprop->BufferSize = sizeof( devprop_basic_val.byte ); + break; + case VT_BOOL: + devprop->Type = DEVPROP_TYPE_BOOLEAN; + devprop_basic_val.boolean = tmp.boolVal ? DEVPROP_TRUE : DEVPROP_FALSE; + devprop->BufferSize = sizeof( devprop_basic_val.boolean ); + break; + case VT_I2: + case VT_UI2: + devprop->Type = prop_vt == VT_I2 ? DEVPROP_TYPE_INT16 : DEVPROP_TYPE_UINT16; + devprop_basic_val.int16 = tmp.uiVal; + devprop->BufferSize = sizeof( devprop_basic_val.int16 ); + break; + case VT_I4: + case VT_UI4: + devprop->Type = prop_vt == VT_I4 ? DEVPROP_TYPE_INT32 : DEVPROP_TYPE_UINT32; + devprop_basic_val.int32 = tmp.ulVal; + devprop->BufferSize = sizeof( devprop_basic_val.int32 ); + break; + case VT_I8: + case VT_UI8: + devprop->Type = prop_vt == VT_I8 ? DEVPROP_TYPE_INT64 : DEVPROP_TYPE_UINT64; + devprop_basic_val.int64 = tmp.uhVal.QuadPart; + devprop->BufferSize = sizeof( devprop_basic_val.int64 ); + break; + case VT_LPWSTR: + devprop->Type = DEVPROP_TYPE_STRING; + devprop->BufferSize = (wcslen( tmp.pwszVal ) + 1) * sizeof( WCHAR ); + break; + default: + FIXME( "Unsupported property VARTYPE %d, treating comparand as string.\n", prop_vt ); + PropVariantClear( &tmp ); + if (FAILED(hr = PropVariantChangeType( &tmp, comparand_val, 0, VT_LPWSTR ))) + return (hr == E_FAIL || hr == E_NOTIMPL) ? E_INVALIDARG : hr; + devprop->Type = DEVPROP_TYPE_STRING; + devprop->BufferSize = (wcslen( tmp.pwszVal ) + 1) * sizeof( WCHAR ); + break; + } + } + else + { + devprop->Type = DEVPROP_TYPE_EMPTY; + devprop->BufferSize = 0; + } + + devprop->CompKey.Store = DEVPROP_STORE_SYSTEM; + devprop->CompKey.LocaleName = NULL; + devprop->Buffer = NULL; + if (devprop->BufferSize && !(devprop->Buffer = calloc( 1, devprop->BufferSize ))) + { + PropVariantClear( &tmp ); + return E_OUTOFMEMORY; + } + switch (devprop->Type) + { + case DEVPROP_TYPE_STRING: + wcscpy( devprop->Buffer, tmp.pwszVal ); + break; + case DEVPROP_TYPE_EMPTY: + break; + default: + memcpy( devprop->Buffer, &devprop_basic_val, devprop->BufferSize ); + break; + } + PropVariantClear( &tmp ); + return S_OK; +} + +static HRESULT filters_append_op( DEVPROP_FILTER_EXPRESSION **filters, ULONG *len, ULONG len_hint, DEVPROP_OPERATOR op ) +{ + DEVPROP_FILTER_EXPRESSION *tmp; + + if (!(tmp = realloc( *filters, sizeof( *tmp ) * (len_hint ? len_hint : (*len + 1)) ))) + return E_OUTOFMEMORY; + + *filters = tmp; + tmp = &(*filters)[*len]; + memset( tmp, 0, sizeof( *tmp ) ); + tmp->Operator = op; + *len += 1; + return S_OK; +} + +static HRESULT devprop_filters_append_expr( const struct aqs_expr *expr, DEVPROP_FILTER_EXPRESSION **filters, ULONG *len ) +{ + static const DEVPROP_OPERATOR boolean_ops[] = { + DEVPROP_OPERATOR_AND_OPEN, + DEVPROP_OPERATOR_OR_OPEN, + DEVPROP_OPERATOR_NOT_OPEN, + }; + static const DEVPROP_OPERATOR compare_ops[] = { + DEVPROP_OPERATOR_EQUALS, + DEVPROP_OPERATOR_NOT_EQUALS, + DEVPROP_OPERATOR_GREATER_THAN, + DEVPROP_OPERATOR_GREATER_THAN_EQUALS, + DEVPROP_OPERATOR_LESS_THAN, + DEVPROP_OPERATOR_LESS_THAN_EQUALS, + DEVPROP_OPERATOR_BEGINS_WITH_IGNORE_CASE, + DEVPROP_OPERATOR_ENDS_WITH_IGNORE_CASE, + DEVPROP_OPERATOR_CONTAINS_IGNORE_CASE, + DEVPROP_OPERATOR_MODIFIER_NOT | DEVPROP_OPERATOR_CONTAINS_IGNORE_CASE, + DEVPROP_OPERATOR_CONTAINS, + }; + HRESULT hr = S_OK; + + if (!expr) return S_OK; + + switch (expr->op_type) + { + case OP_TYPE_BOOLEAN: + { + const struct expr_boolean *boolean = &expr->u.boolean; + DEVPROP_OPERATOR open = boolean_ops[boolean->op]; + DEVPROP_OPERATOR close = open + (DEVPROP_OPERATOR_AND_CLOSE - DEVPROP_OPERATOR_AND_OPEN); + + /* We'll append at least two filters, _OPEN and _CLOSE. */ + if (FAILED(hr = filters_append_op( filters, len, *len + 2, open ))) return hr; + if (FAILED(hr = devprop_filters_append_expr( boolean->lhs, filters, len ))) return hr; + if (FAILED(hr = devprop_filters_append_expr( boolean->rhs, filters, len ))) return hr; + hr = filters_append_op( filters, len, 0, close ); + break; + } + case OP_TYPE_COMPARE: + { + const struct expr_compare *compare = &expr->u.compare; + DEVPROP_OPERATOR op; + + if (compare->op == OP_MATCH_WILDCARD) FIXME( "Wildcard matching is not supported yet, will compare verbatim.\n" ); + + if ((compare->op == OP_EQ || compare->op == OP_NEQ) && compare->val.vt == VT_EMPTY) + op = (compare->op == OP_EQ) ? DEVPROP_OPERATOR_NOT_EXISTS : DEVPROP_OPERATOR_EXISTS; + else + op = compare_ops[compare->op]; + if (FAILED(hr = filters_append_op( filters, len, 0, op ))) return hr; + hr = propval_to_devprop( &compare->val, compare->prop_desc, &(*filters)[*len - 1].Property); + break; + } + } + return hr; +} + +static HRESULT aqs_expr_to_filters( const struct aqs_expr *expr, DEVPROP_FILTER_EXPRESSION **filters, ULONG *filters_len ) +{ + HRESULT hr; + + *filters = NULL; + *filters_len = 0; + if (FAILED(hr = devprop_filters_append_expr( expr, filters, filters_len ))) + { + free_devprop_filters( *filters, *filters_len ); + *filters = NULL; + *filters_len = 0; + } + return hr; +} + +void free_devprop_filters( DEVPROP_FILTER_EXPRESSION *filters, ULONG filters_len ) +{ + ULONG i; + + for (i = 0; i < filters_len; i++) + free( filters[i].Property.Buffer ); + free( filters ); +} diff --git a/dlls/windows.devices.enumeration/aqs.h b/dlls/windows.devices.enumeration/aqs.h index 5f229b2eb6c..5dcc3872e84 100644 --- a/dlls/windows.devices.enumeration/aqs.h +++ b/dlls/windows.devices.enumeration/aqs.h @@ -100,7 +100,7 @@ struct aqs_expr } u; }; -extern HRESULT aqs_parse_query( const WCHAR *str ); +extern HRESULT aqs_parse_query( const WCHAR *str, DEVPROP_FILTER_EXPRESSION **filters, ULONG *filters_len ); extern UINT aqs_lex( void *val, struct aqs_parser *parser ); @@ -113,3 +113,4 @@ extern HRESULT get_compare_expr( struct aqs_parser *parser, enum operator_compar const PROPVARIANT *val, struct aqs_expr **expr ); extern void free_expr( struct aqs_expr *expr ); +extern void free_devprop_filters( DEVPROP_FILTER_EXPRESSION *filters, ULONG filters_len ); diff --git a/dlls/windows.devices.enumeration/main.c b/dlls/windows.devices.enumeration/main.c index ca5af3f2100..667d85e134b 100644 --- a/dlls/windows.devices.enumeration/main.c +++ b/dlls/windows.devices.enumeration/main.c @@ -531,6 +531,81 @@ static HRESULT WINAPI device_statics_CreateFromIdAsyncAdditionalProperties( IDev return E_NOTIMPL; } +struct devquery_params +{ + IUnknown IUnknown_iface; + DEVPROP_FILTER_EXPRESSION *filters; + ULONG filters_len; + LONG ref; +}; + +static inline struct devquery_params *impl_from_IUnknown( IUnknown *iface ) +{ + return CONTAINING_RECORD( iface, struct devquery_params, IUnknown_iface ); +} + +static HRESULT WINAPI devquery_params_QueryInterface( IUnknown *iface, REFIID iid, void **out ) +{ + TRACE( "iface %p, iid %s, out %p\n", iface, debugstr_guid( iid ), out ); + + if (IsEqualGUID( iid, &IID_IUnknown )) + { + IUnknown_AddRef(( iface )); + *out = iface; + return S_OK; + } + + *out = NULL; + FIXME( "%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid( iid ) ); + return S_OK; +} + +static ULONG WINAPI devquery_params_AddRef( IUnknown *iface ) +{ + struct devquery_params *impl = impl_from_IUnknown( iface ); + + TRACE( "iface %p\n", iface ); + return InterlockedIncrement( &impl->ref ); +} + +static ULONG WINAPI devquery_params_Release( IUnknown *iface ) +{ + struct devquery_params *impl = impl_from_IUnknown( iface ); + ULONG ref = InterlockedDecrement( &impl->ref ); + + TRACE( "iface %p\n", iface ); + + if (!ref) + { + free_devprop_filters( impl->filters, impl->filters_len ); + free( impl ); + } + return ref; +} + +static const IUnknownVtbl devquery_params_vtbl = +{ + /* IUnknown */ + devquery_params_QueryInterface, + devquery_params_AddRef, + devquery_params_Release, +}; + +static HRESULT create_devquery_params( DEVPROP_FILTER_EXPRESSION *filters, ULONG filters_len, IUnknown **out ) +{ + struct devquery_params *impl; + + *out = NULL; + if (!(impl = calloc( 1, sizeof( *impl ) ))) return E_OUTOFMEMORY; + + impl->IUnknown_iface.lpVtbl = &devquery_params_vtbl; + impl->ref = 1; + impl->filters = filters; + impl->filters_len = filters_len; + *out = &impl->IUnknown_iface; + return S_OK; +} + static HRESULT find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT *result ) { static const struct vector_iids iids = @@ -541,6 +616,7 @@ static HRESULT find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT * .iterator = &IID_IIterator_DeviceInformation, }; IVectorView_DeviceInformation *view; + struct devquery_params *params; IVector_IInspectable *vector; const DEV_OBJECT *objects; ULONG len, i; @@ -548,8 +624,9 @@ static HRESULT find_all_async( IUnknown *invoker, IUnknown *param, PROPVARIANT * TRACE( "invoker %p, param %p, result %p\n", invoker, param, result ); + params = impl_from_IUnknown( param ); if (FAILED(hr = vector_create( &iids, (void *)&vector ))) return hr; - if (FAILED(hr = DevGetObjects( DevObjectTypeDeviceInterfaceDisplay, DevQueryFlagNone, 0, NULL, 0, NULL, &len, &objects ))) + if (FAILED(hr = DevGetObjects( DevObjectTypeDeviceInterfaceDisplay, DevQueryFlagNone, 0, NULL, params->filters_len, params->filters, &len, &objects ))) { IVector_IInspectable_Release( vector ); return hr; @@ -590,12 +667,20 @@ static HRESULT WINAPI device_statics_FindAllAsyncDeviceClass( IDeviceInformation static HRESULT WINAPI device_statics_FindAllAsyncAqsFilter( IDeviceInformationStatics *iface, HSTRING filter, IAsyncOperation_DeviceInformationCollection **op ) { + DEVPROP_FILTER_EXPRESSION *filters; + ULONG filters_len; + IUnknown *params; HRESULT hr; TRACE( "iface %p, aqs %p, op %p\n", iface, debugstr_hstring(filter), op ); - if (FAILED(hr = aqs_parse_query(WindowsGetStringRawBuffer( filter, NULL )))) return hr; - return async_operation_inspectable_create( &IID_IAsyncOperation_DeviceInformationCollection, (IUnknown *)iface, NULL, + if (FAILED(hr = aqs_parse_query(WindowsGetStringRawBuffer( filter, NULL ), &filters, &filters_len ))) return hr; + if (FAILED(hr = create_devquery_params( filters, filters_len, ¶ms ))) + { + free_devprop_filters( filters, filters_len ); + return hr; + } + return async_operation_inspectable_create( &IID_IAsyncOperation_DeviceInformationCollection, (IUnknown *)iface, (IUnknown *)params, find_all_async, (IAsyncOperation_IInspectable **)op ); } diff --git a/dlls/windows.devices.enumeration/tests/devices.c b/dlls/windows.devices.enumeration/tests/devices.c index 8fbfd9b50af..59567fd71bb 100644 --- a/dlls/windows.devices.enumeration/tests/devices.c +++ b/dlls/windows.devices.enumeration/tests/devices.c @@ -666,8 +666,8 @@ static void test_DeviceInformation_filters( void ) test_FindAllAsyncAqsFilter( statics, simple, FALSE, FALSE ); test_FindAllAsyncAqsFilter( statics, case_insensitive, TRUE, FALSE ); - test_FindAllAsyncAqsFilter( statics, no_results, FALSE, TRUE ); - test_FindAllAsyncAqsFilter( statics, invalid_comparand_type, TRUE, FALSE ); + test_FindAllAsyncAqsFilter( statics, no_results, FALSE, FALSE ); + test_FindAllAsyncAqsFilter( statics, invalid_comparand_type, FALSE, FALSE ); test_FindAllAsyncAqsFilter( statics, invalid_empty, FALSE, FALSE ); test_FindAllAsyncAqsFilter( statics, invalid_operator, FALSE, FALSE ); test_FindAllAsyncAqsFilter( statics, invalid_operand, FALSE, FALSE ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
From: Vibhav Pant <vibhavp(a)gmail.com> --- MAINTAINERS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ad646df417f..7bbdac35021 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -391,6 +391,10 @@ WinINet P: Jacek Caban <jacek(a)codeweavers.com> F: dlls/wininet/ +WinRT Device Enumeration +P: Vibhav Pant <vibhavp(a)gmail.com> +F: dlls/windows.devices.enumeration/ + X11 Driver M: Alexandre Julliard <julliard(a)winehq.org> P: Rémi Bernon <rbernon(a)codeweavers.com> -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8890
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ for (i = 1; str[i]; i++) + { + if (str[i] == '\"') + { + if (str[i+1] && str[i+1] == '\"') + { + i++; + continue; + } + break; + } + } + if (str[i]) + i++; + *token = TK_STRING; + return i; What do you think of this instead?
```suggestion:-15+0 /* lookup for end double quote, skipping any "" escaped double quotes */ for (i = 1; str[i]; i++) if (str[i] == '\"' && str[++i] != '\"') break; if (i == 1 || str[i - 1] != '\"') break; /* illegal */ *token = TK_STRING; return i; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115220
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, + 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1, + 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 0, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, +}; + +static int is_idchar( WCHAR chr ) { return chr >= ARRAY_SIZE( id_char ) || id_char[chr]; } Could we do something more easily readable/modifiable? Seems like just a handful of ranges to check.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115217
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ *token = TK_ILLEGAL; + return 1; + } + *token = TK_NULL; + return 2; + case ':': + *token = TK_COLON; + return 1; + case '=': + *token = TK_EQUAL; + return 1; + case L'\u2260': /* ≠, NOT EQUALS TO */ + *token = TK_NOTEQUAL; + return 1; + case '-': + if (iswdigit(str[1]))
if (iswdigit( str[1] ))
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115218
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ case '\n': + for (i = 1; iswspace( str[i] ); i++); + *token = TK_WHITESPACE; + return i; + case '(': + *token = TK_LEFTPAREN; + return 1; + case ')': + *token = TK_RIGHTPAREN; + return 1; + case '[': + if (str[1] != ']') + { + *token = TK_ILLEGAL; + return 1; + } What about using the TK_ILLEGAL default below?
```suggestion:-4+0 if (str[1] != ']') break; /* illegal */ ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115219
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.y:
+static const PROPVARIANT propval_empty = { VT_EMPTY }; + +static int aqs_error( struct aqs_parser *parser, const char *str ) +{ + if (TRACE_ON( aqs)) ERR( "%s\n", str ); + return 0; +} + +#define GET_COMPARE_EXPR( ctx, op, prop_vt, val_vt, out ) do { \ + HRESULT hr; \ + hr = get_compare_expr( ctx, op, (prop_vt)->pwszVal, val_vt, out ); \ + PropVariantClear( prop_vt ); \ + PropVariantClear( val_vt ); \ + if (FAILED( hr )) \ + YYABORT; \ + } while (0) What about sinking both propvariant ownership into the expr and letting it deal with cleanup?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115226
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.y:
+ ; +expr: + TK_LEFTPAREN expr TK_RIGHTPAREN + { + $$ = $2; +#if YYBISON >= 30704 + (void)yysymbol_name; /* avoid unused function warning */ +#endif + (void)yynerrs; /* avoid unused variable warning */ + } + | expr TK_AND expr + { + HRESULT hr; + hr = get_boolean_expr( ctx, OP_AND, $1, $3, &$$ ); + if (FAILED( hr )) + YYABORT; Doesn't seem you need these hr, IMO you could move the call within the FAILED check.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115225
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ return TK_TRUE; + + return TK_ID; +} + +int get_token( const WCHAR *str, aqs_token_kind_t *token ) +{ + int i; + + switch (str[0]) + { + case ' ': + case '\t': + case '\r': + case '\n': + for (i = 1; iswspace( str[i] ); i++);
for (i = 1; iswspace( str[i] ); i++) /* nothing */;
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115224
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ } + *token = TK_ILLEGAL; + return 1; +} + +UINT aqs_lex( void *p, struct aqs_parser *parser ) +{ + aqs_token_kind_t token = -1; + struct string *str = p; + + do + { + parser->idx += parser->len; + if (!parser->query[parser->idx]) return 0; + if (!(parser->len = get_token( &parser->query[parser->idx], &token ))) + break; Can this ever return 0? Should we return 0 on NUL char or invalid token maybe?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115223
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ } + break; + } + } + if (str[i]) + i++; + *token = TK_STRING; + return i; + case '0': case '1': case '2': case '3': case '4': + case '5': case '6': case '7': case '8': case '9': + *token = TK_INTEGER; + for (i = 1; iswdigit( str[i] ); i++); + return i; + default: + if (!is_idchar( str[0] )) break; + for (i = 1; is_idchar( str[i] ); i++);
for (i = 1; is_idchar( str[i] ); i++) /* nothing */;
Same. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115222
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ if (str[i+1] && str[i+1] == '\"') + { + i++; + continue; + } + break; + } + } + if (str[i]) + i++; + *token = TK_STRING; + return i; + case '0': case '1': case '2': case '3': case '4': + case '5': case '6': case '7': case '8': case '9': + *token = TK_INTEGER; + for (i = 1; iswdigit( str[i] ); i++);
for (i = 1; iswdigit( str[i] ); i++) /* nothing */;
I think we usually add a comment there to avoid missing the empty loop. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115221
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.y:
+ case OP_TYPE_BOOLEAN: + if (expr->u.boolean.op < ARRAY_SIZE( bool_op )) + return wine_dbg_sprintf( "{%s %p %p}", bool_op[expr->u.boolean.op], expr->u.boolean.lhs, + expr->u.boolean.rhs ); + return wine_dbg_sprintf( "{(unknown %d) %p %p}", expr->u.boolean.op, expr->u.boolean.lhs, expr->u.boolean.rhs ); + case OP_TYPE_COMPARE: + { + const struct expr_compare *cmp = &expr->u.compare; + if (expr->u.compare.op < ARRAY_SIZE( compare_op )) + return wine_dbg_sprintf( "{%s %p %s}", compare_op[cmp->op], cmp->prop_desc, debugstr_propvar( &cmp->val ) ); + return wine_dbg_sprintf( "{(unknown %d) %p %s}", cmp->op, cmp->prop_desc, debugstr_propvar( &cmp->val ) ); + } + default: + return wine_dbg_sprintf( "{(unknown %d)}", expr->op_type ); + } +} I think you should move these to aqs.c (or to aqs.h for `debugstr_propvar`), would let you hide most of the expression structures, and reduce friction between the .y and .c. Keeping the .y minimal is better IMO, they often don't benefit from the usual C source editor features.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115227
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ return S_OK; +} + +HRESULT get_compare_expr( struct aqs_parser *parser, enum operator_compare op, const WCHAR *prop_name, + const PROPVARIANT *val, struct aqs_expr **ret_expr ) +{ + struct aqs_expr *expr; + HRESULT hr; + + if (!(expr = calloc( 1, sizeof( *expr )))) return (parser->error = E_OUTOFMEMORY); + if (FAILED(hr = IPropertySystem_GetPropertyDescriptionByName( parser->propsys, prop_name, &IID_IPropertyDescription, (void **)&expr->u.compare.prop_desc))) + { + free( expr ); + parser->error = hr == TYPE_E_ELEMENTNOTFOUND ? E_INVALIDARG : hr; + return parser->error; + } The `compare.prop_desc` member doesn't seem to be used, do you even need to keep it around?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115230
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.h:
+}; + +struct expr_compare +{ + enum operator_compare op; + IPropertyDescription *prop_desc; + PROPVARIANT val; +}; + +struct aqs_expr +{ + enum operator_type op_type; + union { + struct expr_boolean boolean; + struct expr_compare compare; + } u;
};
I think nameless unions are usually preferred. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115231
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+{ + const WCHAR *str = p->data; + SIZE_T len = p->len; + WCHAR *buf; + + memset( val, 0, sizeof( *val ) ); + if (!id) + { + str++; + len -= 2; + } + if (!(buf = CoTaskMemAlloc((len + 1) * sizeof( WCHAR )))) + { + parser->error = E_OUTOFMEMORY; + return E_OUTOFMEMORY; + }
if (!(buf = CoTaskMemAlloc( (len + 1) * sizeof( WCHAR ) ))) return (parser->error = E_OUTOFMEMORY);
You used that pattern below, lets stick to it. Same elsewhere. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115228
Rémi Bernon (@rbernon) commented about dlls/windows.devices.enumeration/aqs.c:
+ + memset( val, 0, sizeof(*val) ); + for (i = 0; i < parser->len; i++) + num = (str[i] - '0') + num * 10; + val->vt = VT_UI4; + val->lVal = num; + return S_OK; +} + +HRESULT get_string( struct aqs_parser *parser, const struct string *p, BOOL id, PROPVARIANT *val ) +{ + const WCHAR *str = p->data; + SIZE_T len = p->len; + WCHAR *buf; + + memset( val, 0, sizeof( *val ) ); Is this even necessary?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115229
On Mon Sep 8 09:45:58 2025 +0000, Rémi Bernon wrote:
Can this ever return 0? Should we return 0 on NUL char or invalid token maybe? Yes, I have moved the NUL/EOF check to `get_token`. Also, Bison already defines `AQS_UNDEF`, so TK_ILLEGAL isn't needed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115264
On Mon Sep 8 09:45:57 2025 +0000, Rémi Bernon wrote:
Could we do something more easily readable/modifiable? Seems like just a handful of ranges to check. I've rewritten to be more readable, is it better?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115288
On Mon Sep 8 09:45:57 2025 +0000, Rémi Bernon wrote:
What do you think of this instead? ```suggestion:-15+0 /* lookup for end double quote, skipping any "" escaped double quotes */ for (i = 1; str[i]; i++) if (str[i] == '\"' && str[++i] != '\"') break; if (i == 1 || str[i - 1] != '\"') break; /* illegal */ *token = TK_STRING; return i; ``` Yes, that's simpler. Thanks :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115289
On Mon Sep 8 09:45:58 2025 +0000, Rémi Bernon wrote:
What about sinking both propvariant ownership into the expr and letting it deal with cleanup? Sure, thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115291
On Mon Sep 8 09:45:58 2025 +0000, Rémi Bernon wrote:
I think you should move these to aqs.c (or to aqs.h for `debugstr_propvar`), would let you hide most of the expression structures, and reduce friction between the .y and .c. Keeping the .y minimal is better IMO, they often don't benefit from the usual C source editor features. Yes, it's easier to edit them from .c code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115292
On Mon Sep 8 09:45:59 2025 +0000, Rémi Bernon wrote:
Is this even necessary? Nope, removed. Thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115293
On Mon Sep 8 09:45:59 2025 +0000, Rémi Bernon wrote:
The `compare.prop_desc` member doesn't seem to be used, do you even need to keep it around? This got used in the next commit for `IPropertyDescription_GetPropertyKey` and `GetPropertyType`. I've rewritten this to get the `DEVPROPKEY` and `VARTYPE` here itself and store them inside `struct expr_compare`, which obviates the need to keep a reference to the `IPropertyDescription` object. Thanks.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8890#note_115294
participants (3)
-
Rémi Bernon -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)