On Sun, May 3, 2009 at 1:52 PM, Mike Kaplinskiy <mike.kaplins...@gmail.com> wrote: > On Thu, Apr 30, 2009 at 1:08 PM, James Hawkins <trui...@gmail.com> wrote: >> On Thu, Apr 30, 2009 at 4:03 AM, Austin English <austinengl...@gmail.com> >> wrote: >>> On Tue, Apr 28, 2009 at 9:27 PM, Mike Kaplinskiy >>> <mike.kaplins...@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) >
I'll take a look at your patch. No offense intended, but it's probably not correct. The problem is a beast and deeply rooted in the way we store temporary rows in the DB. I have a huge test patch for it, and I had a fix, but my linux HD got fragged (bummer). -- James Hawkins