Office 2007 MSI Crash - Null dereference @ MsiViewExecute

[Mike Kaplinskiy]

On Thu, Apr 30, 2009 at 1:08 PM, James Hawkins <truiken at gmail.com> wrote:
> On Thu, Apr 30, 2009 at 4:03 AM, Austin English <austinenglish at gmail.com> wrote:
>> On Tue, Apr 28, 2009 at 9:27 PM, Mike Kaplinskiy
>> <mike.kaplinskiy at gmail.com> wrote:
>>> I was looking at the trace of the crash from bug 17600, and it looks like
>>> a custom action is calling MsiViewExecute with a null hRec.
>>>
>>> I (sadly) don't know much about the wine MSI architecture, but the
>>> msiobj_lock on line 484 should fail since rec will never be fetched
>>> (null). I think the intention was to make it query->hdr (as it is
>>> released later).
>>
>> A testcase for it would show if you're right or wrong ;-).
>>
>
> Not really.  If you grep through the msi tests, you'll see that we
> call MsiViewExecute with NULL hRec all over the place.  That doesn't
> mean there isn't a bug, just saying.
>
> --
> James Hawkins
>

So I got a little time to look into this, and was completely blown
away. Apparently doing

TRACE("dereferencing 0?=%p\n", (void *) &(((MSIRECORD *) NULL)->hdr));

works, since nothing ever gets dereferenced. I will post a patch for
this small nit.

On a slightly different note, I have traced the problem in office 2007
to the patch by James, and in particular to INSERT_execute. The row
index gets set to 0 if the primary key is null (which is what office
install is doing), but if the insert is temporary TABLE_insert assumes
that row >= tv->table->row_count, and subtracts from the row index,
putting us in the negatives (or high positives due to uint).

I have a patch for this, but I don't know how to make a testcase (all
I know is office installs), so would anyone mind pointing me to
somewhere that explains msi temporary inserts?

And should I post the patch up on wine-patches to get it critiqued?
(sorry, first time)

Mike.