[PATCH 0/4] MR8065: msado15: Make Fields part of rowset structure.
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msado15/recordset.c | 5 +++-- dlls/msado15/tests/msado15.c | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/msado15/recordset.c b/dlls/msado15/recordset.c index dd8e15cbe86..6454b032b26 100644 --- a/dlls/msado15/recordset.c +++ b/dlls/msado15/recordset.c @@ -2077,7 +2077,7 @@ static HRESULT WINAPI recordset_Open( _Recordset *iface, VARIANT source, VARIANT if (recordset->state == adStateOpen) return MAKE_ADO_HRESULT( adErrObjectOpen ); - if (recordset->fields) + if (recordset->fields && get_column_count( recordset )) { recordset->state = adStateOpen; return S_OK; @@ -2122,7 +2122,8 @@ static HRESULT WINAPI recordset_Open( _Recordset *iface, VARIANT source, VARIANT /* We want to create the field member variable without mapping the rowset fields, this will * save querying the fields twice. Fields will be added while we create the bindings. */ - hr = fields_create( recordset, &recordset->fields ); + if (!recordset->fields) + hr = fields_create( recordset, &recordset->fields ); if (FAILED(hr)) { IUnknown_Release(rowset); diff --git a/dlls/msado15/tests/msado15.c b/dlls/msado15/tests/msado15.c index 76e1bb35b77..66dd69a144e 100644 --- a/dlls/msado15/tests/msado15.c +++ b/dlls/msado15/tests/msado15.c @@ -186,6 +186,11 @@ static void test_Recordset(void) hr = _Recordset_get_Fields( recordset, &fields ); ok( hr == S_OK, "got %08lx\n", hr ); + V_VT( &missing ) = VT_ERROR; + V_ERROR( &missing ) = DISP_E_PARAMNOTFOUND; + hr = _Recordset_Open( recordset, missing, missing, adOpenStatic, adLockBatchOptimistic, adCmdUnspecified ); + ok( hr == MAKE_ADO_HRESULT( adErrInvalidConnection ), "got %08lx\n", hr ); + name = SysAllocString( L"field" ); hr = Fields__Append( fields, name, adInteger, 4, adFldUnspecified ); ok( hr == S_OK, "got %08lx\n", hr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8065
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msado15/recordset.c | 3 +++ dlls/msado15/tests/msado15.c | 8 ++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/dlls/msado15/recordset.c b/dlls/msado15/recordset.c index 6454b032b26..704ffcac954 100644 --- a/dlls/msado15/recordset.c +++ b/dlls/msado15/recordset.c @@ -2744,6 +2744,9 @@ static HRESULT WINAPI rsconstruction_put_Rowset(ADORecordsetConstruction *iface, if ( recordset->row_set ) IRowset_Release( recordset->row_set ); recordset->row_set = rowset; + if ( recordset->fields && !get_column_count(recordset) ) + map_rowset_fields(recordset, recordset->fields); + return S_OK; } diff --git a/dlls/msado15/tests/msado15.c b/dlls/msado15/tests/msado15.c index 66dd69a144e..45d6652b346 100644 --- a/dlls/msado15/tests/msado15.c +++ b/dlls/msado15/tests/msado15.c @@ -720,6 +720,10 @@ static void test_ADORecordsetConstruction(void) goto done; } + hr = _Recordset_get_Fields( recordset, &fields ); + ok( hr == S_OK, "got %08lx\n", hr ); + ok( fields != NULL, "NULL value\n"); + testrowset.IRowset_iface.lpVtbl = &rowset_vtbl; testrowset.IRowsetInfo_iface.lpVtbl = &rowset_info; testrowset.IColumnsInfo_iface.lpVtbl = &column_info; @@ -735,10 +739,6 @@ static void test_ADORecordsetConstruction(void) ref = get_refcount( rowset ); ok( ref == 2, "got %ld\n", ref ); - hr = _Recordset_get_Fields( recordset, &fields ); - ok( hr == S_OK, "got %08lx\n", hr ); - ok( fields != NULL, "NULL value\n"); - ref = get_refcount( rowset ); ok( ref == 2, "got %ld\n", ref ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8065
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msado15/recordset.c | 111 ++++++++++++++------------------------- 1 file changed, 39 insertions(+), 72 deletions(-) diff --git a/dlls/msado15/recordset.c b/dlls/msado15/recordset.c index 704ffcac954..66309407268 100644 --- a/dlls/msado15/recordset.c +++ b/dlls/msado15/recordset.c @@ -32,7 +32,16 @@ WINE_DEFAULT_DEBUG_CHANNEL(msado15); -struct fields; +struct fields +{ + Fields Fields_iface; + ISupportErrorInfo ISupportErrorInfo_iface; + LONG refs; + Field **field; + ULONG count; + ULONG allocated; +}; + struct recordset { _Recordset Recordset_iface; @@ -41,7 +50,7 @@ struct recordset LONG refs; VARIANT active_connection; LONG state; - struct fields *fields; + struct fields fields; LONG count; LONG allocated; LONG index; @@ -58,17 +67,6 @@ struct recordset HACCESSOR *haccessors; }; -struct fields -{ - Fields Fields_iface; - ISupportErrorInfo ISupportErrorInfo_iface; - LONG refs; - Field **field; - ULONG count; - ULONG allocated; - struct recordset *recordset; -}; - struct field { Field Field_iface; @@ -258,7 +256,7 @@ static HRESULT WINAPI field_get_Type( Field *iface, DataTypeEnum *type ) static LONG get_column_count( struct recordset *recordset ) { - return recordset->fields->count; + return recordset->fields.count; } static HRESULT WINAPI field_get_Value( Field *iface, VARIANT *val ) @@ -813,11 +811,19 @@ static inline struct fields *impl_from_Fields( Fields *iface ) return CONTAINING_RECORD( iface, struct fields, Fields_iface ); } +static inline struct recordset *fields_get_recordset( struct fields *fields ) +{ + return CONTAINING_RECORD( fields, struct recordset, fields ); +} + static ULONG WINAPI fields_AddRef( Fields *iface ) { struct fields *fields = impl_from_Fields( iface ); LONG refs = InterlockedIncrement( &fields->refs ); + TRACE( "%p new refcount %ld\n", fields, refs ); + + if (refs == 1) _Recordset_AddRef( &fields_get_recordset(fields)->Recordset_iface ); return refs; } @@ -825,15 +831,11 @@ static ULONG WINAPI fields_Release( Fields *iface ) { struct fields *fields = impl_from_Fields( iface ); LONG refs = InterlockedDecrement( &fields->refs ); + TRACE( "%p new refcount %ld\n", fields, refs ); - if (!refs) - { - if (fields->recordset) _Recordset_Release( &fields->recordset->Recordset_iface ); - fields->recordset = NULL; - WARN( "not destroying %p\n", fields ); - return InterlockedIncrement( &fields->refs ); - } - return refs; + + if (!refs) _Recordset_Release( &fields_get_recordset(fields)->Recordset_iface ); + return refs < 1 ? 1 : refs; } static HRESULT WINAPI fields_QueryInterface( Fields *iface, REFIID riid, void **obj ) @@ -1029,7 +1031,7 @@ static HRESULT append_field( struct fields *fields, BSTR name, DataTypeEnum type Field *field; HRESULT hr; - if ((hr = Field_create( name, fields->count, fields->recordset, &field )) != S_OK) return hr; + if ((hr = Field_create( name, fields->count, fields_get_recordset(fields), &field )) != S_OK) return hr; Field_put_Type( field, type ); Field_put_DefinedSize( field, size ); if (attr != adFldUnspecified) Field_put_Attributes( field, attr ); @@ -1187,23 +1189,11 @@ static void map_rowset_fields(struct recordset *recordset, struct fields *fields IColumnsInfo_Release(columninfo); } -static HRESULT fields_create( struct recordset *recordset, struct fields **ret ) +static void fields_init( struct recordset *recordset ) { - struct fields *fields; - - if (!(fields = calloc( 1, sizeof(*fields) ))) return E_OUTOFMEMORY; - fields->Fields_iface.lpVtbl = &fields_vtbl; - fields->ISupportErrorInfo_iface.lpVtbl = &fields_supporterrorinfo_vtbl; - fields->refs = 1; - fields->recordset = recordset; - _Recordset_AddRef( &fields->recordset->Recordset_iface ); - - if ( recordset->row_set ) - map_rowset_fields(recordset, fields); - - *ret = fields; - TRACE( "returning %p\n", *ret ); - return S_OK; + memset( &recordset->fields, 0, sizeof(recordset->fields) ); + recordset->fields.Fields_iface.lpVtbl = &fields_vtbl; + recordset->fields.ISupportErrorInfo_iface.lpVtbl = &fields_supporterrorinfo_vtbl; } static inline struct recordset *impl_from_Recordset( _Recordset *iface ) @@ -1238,14 +1228,13 @@ static void close_recordset( struct recordset *recordset ) VariantClear( &recordset->filter ); - if (!recordset->fields) return; col_count = get_column_count( recordset ); free(recordset->columntypes); for (i = 0; i < col_count; i++) { - struct field *field = impl_from_Field( recordset->fields->field[i] ); + struct field *field = impl_from_Field( recordset->fields.field[i] ); field->recordset = NULL; Field_Release(&field->Field_iface); @@ -1259,9 +1248,7 @@ static void close_recordset( struct recordset *recordset ) free(recordset->haccessors); recordset->haccessors = NULL; } - recordset->fields->count = 0; - Fields_Release( &recordset->fields->Fields_iface ); - recordset->fields = NULL; + recordset->fields.count = 0; for (row = 0; row < recordset->count; row++) for (col = 0; col < col_count; col++) VariantClear( &recordset->data[row * col_count + col] ); @@ -1280,6 +1267,7 @@ static ULONG WINAPI recordset_Release( _Recordset *iface ) { TRACE( "destroying %p\n", recordset ); close_recordset( recordset ); + free( recordset->fields.field ); free( recordset ); } return refs; @@ -1500,22 +1488,11 @@ static HRESULT WINAPI recordset_get_EOF( _Recordset *iface, VARIANT_BOOL *eof ) static HRESULT WINAPI recordset_get_Fields( _Recordset *iface, Fields **obj ) { struct recordset *recordset = impl_from_Recordset( iface ); - HRESULT hr; TRACE( "%p, %p\n", recordset, obj ); - if (recordset->fields) - { - /* yes, this adds a reference to the recordset instead of the fields object */ - _Recordset_AddRef( &recordset->Recordset_iface ); - recordset->fields->recordset = recordset; - *obj = &recordset->fields->Fields_iface; - return S_OK; - } - - if ((hr = fields_create( recordset, &recordset->fields )) != S_OK) return hr; - - *obj = &recordset->fields->Fields_iface; + *obj = &recordset->fields.Fields_iface; + Fields_AddRef(*obj); return S_OK; } @@ -1789,7 +1766,7 @@ static HRESULT create_bindings(IUnknown *rowset, struct recordset *recordset, DB colinfo[i].dwFlags, colinfo[i].ulColumnSize, colinfo[i].wType, colinfo[i].bPrecision, colinfo[i].bScale); - hr = append_field(recordset->fields, colinfo[i].pwszName, colinfo[i].wType, colinfo[i].ulColumnSize, + hr = append_field(&recordset->fields, colinfo[i].pwszName, colinfo[i].wType, colinfo[i].ulColumnSize, colinfo[i].dwFlags, NULL); bindings[i].iOrdinal = colinfo[i].iOrdinal; @@ -2077,7 +2054,7 @@ static HRESULT WINAPI recordset_Open( _Recordset *iface, VARIANT source, VARIANT if (recordset->state == adStateOpen) return MAKE_ADO_HRESULT( adErrObjectOpen ); - if (recordset->fields && get_column_count( recordset )) + if (get_column_count( recordset )) { recordset->state = adStateOpen; return S_OK; @@ -2119,17 +2096,6 @@ static HRESULT WINAPI recordset_Open( _Recordset *iface, VARIANT source, VARIANT if (FAILED(hr) || !rowset) return hr; - /* We want to create the field member variable without mapping the rowset fields, this will - * save querying the fields twice. Fields will be added while we create the bindings. - */ - if (!recordset->fields) - hr = fields_create( recordset, &recordset->fields ); - if (FAILED(hr)) - { - IUnknown_Release(rowset); - return hr; - } - hr = create_bindings(rowset, recordset, &bindings, &datasize); if (FAILED(hr)) { @@ -2744,8 +2710,8 @@ static HRESULT WINAPI rsconstruction_put_Rowset(ADORecordsetConstruction *iface, if ( recordset->row_set ) IRowset_Release( recordset->row_set ); recordset->row_set = rowset; - if ( recordset->fields && !get_column_count(recordset) ) - map_rowset_fields(recordset, recordset->fields); + if ( !get_column_count(recordset) ) + map_rowset_fields(recordset, &recordset->fields); return S_OK; } @@ -2816,6 +2782,7 @@ HRESULT Recordset_create( void **obj ) VariantInit( &recordset->filter ); recordset->columntypes = NULL; recordset->haccessors = NULL; + fields_init( recordset ); *obj = &recordset->Recordset_iface; TRACE( "returning iface %p\n", *obj ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8065
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msado15/tests/msado15.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/dlls/msado15/tests/msado15.c b/dlls/msado15/tests/msado15.c index 45d6652b346..b7134ee4986 100644 --- a/dlls/msado15/tests/msado15.c +++ b/dlls/msado15/tests/msado15.c @@ -83,6 +83,7 @@ static void test_Recordset(void) hr = _Recordset_get_Fields( recordset, &fields2 ); ok( hr == S_OK, "got %08lx\n", hr ); ok( fields2 == fields, "expected same object\n" ); + Fields_Release( fields ); count = -1; hr = Fields_get_Count( fields2, &count ); @@ -93,6 +94,7 @@ static void test_Recordset(void) ok( hr == MAKE_ADO_HRESULT( adErrObjectClosed ), "got %08lx\n", hr ); Fields_Release( fields2 ); + ok( !_Recordset_Release( recordset ), "_Recordset not released\n" ); hr = CoCreateInstance( &CLSID_Recordset, NULL, CLSCTX_INPROC_SERVER, &IID__Recordset, (void **)&recordset ); ok( hr == S_OK, "got %08lx\n", hr ); @@ -434,9 +436,9 @@ if (0) ok( hr == S_OK, "got %08lx\n", hr ); ok( state == adStateClosed, "got %ld\n", state ); - Field_Release( field ); + ok (!Field_Release( field ), "Field not released\n" ); Fields_Release( fields ); - _Recordset_Release( recordset ); + ok( !_Recordset_Release( recordset ), "_Recordset not released\n" ); } /* This interface is queried for but is not documented anywhere. */ @@ -715,10 +717,6 @@ static void test_ADORecordsetConstruction(void) hr = _Recordset_QueryInterface( recordset, &IID_ADORecordsetConstruction, (void**)&construct ); ok( hr == S_OK, "got %08lx\n", hr ); - if (FAILED(hr)) - { - goto done; - } hr = _Recordset_get_Fields( recordset, &fields ); ok( hr == S_OK, "got %08lx\n", hr ); @@ -777,8 +775,7 @@ static void test_ADORecordsetConstruction(void) ADORecordsetConstruction_Release(construct); -done: - _Recordset_Release( recordset ); + ok( !_Recordset_Release( recordset ), "_Recordset not released\n" ); } static void test_Fields(void) @@ -873,7 +870,7 @@ static void test_Fields(void) Field_Release( field ); Fields_Release( fields ); - _Recordset_Release( recordset ); + ok( !_Recordset_Release( recordset ), "_Recordset not released\n" ); } static HRESULT str_to_byte_array( const char *data, VARIANT *ret ) @@ -1253,6 +1250,7 @@ if (0) /* Crashes on windows */ hr = _Connection_get_Provider(connection, &str); ok(hr == S_OK, "Failed, hr 0x%08lx\n", hr); ok(!wcscmp(str, L"MSDASQL.1"), "wrong string %s\n", wine_dbgstr_w(str)); + SysFreeString(str); /* Restore default */ str = SysAllocString(L"MSDASQL"); @@ -1262,7 +1260,6 @@ if (0) /* Crashes on windows */ hr = _Connection_put_Provider(connection, NULL); ok(hr == MAKE_ADO_HRESULT(adErrInvalidArgument), "got 0x%08lx\n", hr); - SysFreeString(str); str = (BSTR)0xdeadbeef; hr = _Connection_get_ConnectionString(connection, &str); @@ -1291,6 +1288,7 @@ if (0) /* Crashes on windows */ hr = _Connection_get_ConnectionString(connection, &str2); ok(hr == S_OK, "Failed, hr 0x%08lx\n", hr); ok(!wcscmp(str, str2), "wrong string %s\n", wine_dbgstr_w(str2)); + SysFreeString(str2); hr = _Connection_Open(connection, NULL, NULL, NULL, 0); ok(hr == E_FAIL, "Failed, hr 0x%08lx\n", hr); @@ -1304,15 +1302,16 @@ if (0) /* Crashes on windows */ hr = _Connection_get_ConnectionString(connection, &str2); ok(hr == S_OK, "Failed, hr 0x%08lx\n", hr); todo_wine ok(!wcscmp(str3, str2) || broken(!wcscmp(str, str2)) /* XP */, "wrong string %s\n", wine_dbgstr_w(str2)); + SysFreeString(str2); hr = _Connection_Open(connection, str, NULL, NULL, adConnectUnspecified); ok(hr == E_FAIL, "Failed, hr 0x%08lx\n", hr); - SysFreeString(str); str2 = NULL; hr = _Connection_get_ConnectionString(connection, &str2); ok(hr == S_OK, "Failed, hr 0x%08lx\n", hr); todo_wine ok(!wcscmp(str3, str2) || broken(!wcscmp(str, str2)) /* XP */, "wrong string %s\n", wine_dbgstr_w(str2)); + SysFreeString(str); SysFreeString(str2); SysFreeString(str3); @@ -1323,7 +1322,7 @@ if (0) /* Crashes on windows */ hr = _Connection_get_ConnectionString(connection, &str); ok(hr == S_OK, "Failed, hr 0x%08lx\n", hr); ok(str == NULL, "got %p\n", str); - _Connection_Release(connection); + ok(!_Connection_Release(connection), "_Connection not released\n"); } static void test_Command(void) @@ -1435,7 +1434,7 @@ static void test_Command(void) Parameters_Release(parameters); Parameters_Release(parameters2); - _Command_Release( command ); + ok( !_Command_Release( command ), "_Command not released\n" ); } struct conn_event { @@ -1644,7 +1643,7 @@ static void test_ConnectionPoint(void) refs = IConnectionPoint_Release( point ); ok( refs == 1, "got %lu", refs ); - IConnectionPointContainer_Release( pointcontainer ); + ok( !IConnectionPointContainer_Release( pointcontainer ), "IConnectionPointContainer not released\n" ); ok( !conn_event.refs, "got %ld\n", conn_event.refs ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8065
participants (2)
-
Piotr Caban -
Piotr Caban (＠piotr)