[PATCH 0/4] MR8065: msado15: Make Fields part of rowset structure.

[PATCH 1/4] msado15: Don't return early in _Recordset_Open if there are no columns.

[Piotr Caban]

From: Piotr Caban

---
 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 );

[PATCH 2/4] msado15: Initialize columns in ADORecordsetConstruction_put_Rowset.

[Piotr Caban]

From: Piotr Caban

---
 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 );

[PATCH 3/4] msado15: Fix Fields object refcounting.

[Piotr Caban]

From: Piotr Caban

---
 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 );

[PATCH 4/4] msado15/tests: Fix some memory leaks.

[Piotr Caban]

From: Piotr Caban

---
 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 );
 }