Re: MSI: partially implement of AppSearch action

[Aric Stewart]

Hi Juan,

YAY! someone else doing action work.. However there are a few problems 
i want to point out so you can review your code and check.

I have attached a patch i quickly made to avoid some problems i was 
having. But what you will want to look over and figure out is

a) if the action returns anything other than ERROR_SUCCESS the install 
will halt at that action returning that error. So you need to make sure 
that you only return errors that should fully halt the install.

b) Watch out for null fields. load_dynamic_stringW will return a NULL 
pointer for those but MSI_RecordGetInteger returns a special value.

oh and i fixed that looks like a copy and paste error.

-aric

Juan Lang wrote:
> ChangeLog: partially implement AppSearch action
>
> --Juan

Index: appsearch.c
===================================================================
RCS file: /cvstrees/crossover/office/wine/dlls/msi/appsearch.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 appsearch.c
--- appsearch.c	1 Feb 2005 14:29:10 -0000	1.1.1.1
+++ appsearch.c	2 Feb 2005 02:44:15 -0000
@@ -127,7 +127,7 @@
         maxVersion = load_dynamic_stringW(row,4);
         if (maxVersion)
         {
-            ACTION_VerStrToInteger(minVersion, &sig->MaxVersionMS,
+            ACTION_VerStrToInteger(maxVersion, &sig->MaxVersionMS,
              &sig->MaxVersionLS);
             HeapFree(GetProcessHeap(), 0, maxVersion);
         }
@@ -610,6 +610,9 @@
     }
     else
         rc = ERROR_OUTOFMEMORY;
+
+    if (rc != ERROR_OUTOFMEMORY )
+        rc = ERROR_SUCCESS;
     return rc;
 }

@@ -689,7 +692,10 @@
             ERR("Error is %x\n",rc);
             goto end;
         }
-        depth = MSI_RecordGetInteger(row,4);
+        if (MSI_RecordIsNull(row,4))
+            depth = 0;
+        else
+            depth = MSI_RecordGetInteger(row,4);
         ACTION_ExpandAnyPath(package, buffer, expanded,
          sizeof(expanded) / sizeof(expanded[0]));
         if (sig->File)

MSI: partially implement of AppSearch action

[Juan Lang]

Hey Aric!

--- Aric Stewart  wrote:
> a) if the action returns anything other than ERROR_SUCCESS the install 
> will halt at that action returning that error. So you need to make sure 
> that you only return errors that should fully halt the install.

Ah, okay, you're right, most of these shouldn't halt the install.  I'll
take a closer look and fix these.
> b) Watch out for null fields. load_dynamic_stringW will return a NULL 
> pointer for those but MSI_RecordGetInteger returns a special value.

I missed that one, thanks for catching it.
> oh and i fixed that looks like a copy and paste error.

Indeed.

Thanks for reviewing.  Go ahead and submit to wine-patches, I think your
changes look good.  I'll take a closer look at the return codes and make
sure they're used appropriately.

--Juan

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com