[PATCH 0/2] MR6326: msi: Fix row index calculation when stringtable contains empty slots.
Native msi deletes strings if no longer used. It's possible to create string table with empty slots this way. In such case we may add new row at incorrect index causing the table to be not sorted by primary key. This affects 3ds Max 2024 installers that contain such string table. I don't see how to add a test case for that without adding precreated MSI file. In order to create such file in tests we will need to implement strings refcounting (that is currently mostly missing). It looks like a big task that will likely introduce regressions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6326
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msi/msipriv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/msi/msipriv.h b/dlls/msi/msipriv.h index e5612331d60..70d2a4d6705 100644 --- a/dlls/msi/msipriv.h +++ b/dlls/msi/msipriv.h @@ -786,7 +786,7 @@ extern UINT msi_commit_streams( MSIDATABASE *db ); /* string table functions */ -extern BOOL msi_add_string( string_table *st, const WCHAR *data, int len, BOOL persistent ); +extern int msi_add_string( string_table *st, const WCHAR *data, int len, BOOL persistent ); extern UINT msi_string2id( const string_table *st, const WCHAR *data, int len, UINT *id ); extern VOID msi_destroy_stringtable( string_table *st ); extern const WCHAR *msi_string_lookup( const string_table *st, UINT id, int *len ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6326
From: Piotr Caban <piotr(a)codeweavers.com> --- dlls/msi/table.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/dlls/msi/table.c b/dlls/msi/table.c index ea4ef3b0d0e..263f0778460 100644 --- a/dlls/msi/table.c +++ b/dlls/msi/table.c @@ -1680,12 +1680,29 @@ static int compare_record( struct table_view *tv, UINT row, MSIRECORD *rec ) return 1; } -static int find_insert_index( struct table_view *tv, MSIRECORD *rec ) +static int find_insert_index( struct table_view *tv, MSIRECORD *rec, BOOL temporary ) { int idx, c, low = 0, high = tv->table->row_count - 1; TRACE("%p %p\n", tv, rec); + for (c = 0; c < tv->num_cols; c++) + { + UINT ival; + + if (!(tv->columns[c].type & MSITYPE_KEY)) continue; + + /* Add primary key string - its index affects row index. */ + if (tv->columns[c].type & MSITYPE_STRING && + (get_table_value_from_record( tv, rec, c + 1, &ival ) == ERROR_NOT_FOUND)) + { + int len; + const WCHAR *sval = msi_record_get_string( rec, c + 1, &len ); + msi_add_string( tv->db->strings, sval, len, !temporary ); + } + break; + } + while (low <= high) { idx = (low + high) / 2; @@ -1718,7 +1735,7 @@ static UINT TABLE_insert_row( struct tagMSIVIEW *view, MSIRECORD *rec, UINT row, return ERROR_FUNCTION_FAILED; if (row == -1) - row = find_insert_index( tv, rec ); + row = find_insert_index( tv, rec, temporary ); r = table_create_new_row( view, &row, temporary ); TRACE("insert_row returned %08x\n", r); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6326
Inserting strings does have the potential to create differences with native when the database is persisted, but that's probably harmless. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6326#note_79396
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6326
On Wed Aug 21 09:31:06 2024 +0000, Hans Leidekker wrote:
Inserting strings does have the potential to create differences with native when the database is persisted, but that's probably harmless. The string needs to be inserted in order to add the row. It just moves the insertion earlier in the code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6326#note_79398
participants (3)
-
Hans Leidekker (＠hans) -
Piotr Caban
-
Piotr Caban (＠piotr)