Re: [try3, 1/4] msi: Added handlers for JScript/VBScript actions that call one script function.

[James Hawkins]

On 3/1/07, Misha Koshelev  wrote:
> I am resending the entire patchset as James Hawkins suggested. The only
> changes from my previous patchset are in patch #2 (add full IDL from
> Windows Installer 3.0 vs 2.0 in the old patchset) and patch #3 (added
> more proper variant handling using GetDispId as per Rob's suggestion and
> removed unnecessary comments as per James' suggestion).
>
> Just as a reminder, these patches add JScript/VBScript support for
> MSI (as the MSI specs specifically state that applications that need
> this functionality must install Windows Script themselves) and partial
> but easily expandable OLE automation functionality for MSI. As a proof
> of concept, sufficient automation support is added to fix bug #7357.
>
> Patch #1 creates custom action handlers for all 8 script custom action
> types using a framework similar to that used for calling DLL functions.
> These custom action handlers all converge on one function that is passed
> a script and a function to be called (if any), and it outputs these in a
> FIXME.
>
> Changelog:
>     * msi: Added handlers for JScript/VBScript actions that call one script
>
> function.

+static DWORD WINAPI ACTION_CallScript( const LPGUID guid )
+{
+    msi_custom_action_info *info;
+    MSIHANDLE hPackage;
+    UINT r = ERROR_FUNCTION_FAILED;
+
+    info = find_action_by_guid( guid );
+    if (!info)
+    {
+        ERR("failed to find action %s\n", debugstr_guid( guid) );
+        return r;
+    }
+
+    FIXME("function %s, script %s\n", debugstr_w( info->function ),
debugstr_w( info->dllname ) );
+
+    hPackage = alloc_msihandle( &info->package->hdr );
+    if (hPackage)
+    {
+	r = S_OK;
+        MsiCloseHandle( hPackage );
+    }
+    else
+        ERR("failed to create handle for %p\n", info->package );
+
+    return r;
+}

Looks like you're leaking info.  If you're going to go down this
route, leaving a stub ACTION_CallScript, then just print the FIXME and
be on your way.  Each patch should be atomic, and assuming this patch
gets applied but none of the others do, you're left with some weird
code that allocates a handle then frees it for no apparent reason.

+    r = MSI_RecordReadStream(row, 2, buffer, &sz);
+    if (r != ERROR_SUCCESS)
+    {
+	msi_free(buffer);
+	return r;
+    }
+
+    buffer[sz] = 0;
+    szw = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, buffer, -1, bufferw, 0);
+    bufferw = msi_alloc(sizeof(WCHAR)*szw);
+    if (!szw)
+    {
+	msi_free(bufferw);
+	return ERROR_FUNCTION_FAILED;
+    }
+
+    r = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, buffer, -1, bufferw, szw);
+    msi_free(buffer);
+    if (!r)
+    {
+	msi_free(bufferw);
+	return ERROR_FUNCTION_FAILED;
+    }

This would be a lot cleaner if you used a 'goto done;' at each of
these error points.  Also, is there a reason you're not using
strdupAtoW?

done:
    msi_free(bufferw)
    ...
    return r;