173821 - Modify the Palm Installer to search for Palm Dlls on local disk

Status: VERIFIED FIXED

(SeaMonkey :: Installer, defect, P2)

Description

[Rajiv Dayal]

There are two binary files (CondMgr.dll and HsApi.dll), required by the Palm
Conduit Installer, which are provided by Palm and Palm provides free
redistribution license for them. However since these binaries are not "open
source" it is advisable to rather not use them for redistribution as part of
Mozilla. 

The install code loads redistributable copies of these binaries to find the Palm
software (HotSync stuff) path and then install the palm conduit, the right thing
to do as per Palm documentation. But we need to change this.

If the user has a Palm handheld and installs the HotSync stuff that comes with
it, it is very likely that these binaries will exist on the users machine.
I have found a Registry key that seems to exist on machines where the Palm
software is installed. I have changed the code to use this registry key to find
the Palm software path and load these 2 dlls from the Palm software location
thus avoiding the need to redistribute them.

Comment 1

[Rajiv Dayal]

Also the PalmSync support module is not yet added in the installer manifest and
registration of Mozilla Palm sync support proxy is currently needed to be done
by hand. So the first needs to be done and the Conduit Installer needs to do
registration and unregistration of support proxy during conduit install and
un-install.

Comment 2

[Rajiv Dayal]

Attachment: patch to take care of all 3 issues above

patch takes care of:
1. Tries to locate Palm binaries(CondMgr.dll, HsApi.dll) using registry key
setting
2. Make palmsync support module part of install manifest
3. Do registration of palmsync support module from the Conduit Installer.

Comment 3

[Rajiv Dayal]

Sean can u please review this patch above.

Comment 4

[Rajiv Dayal]

Changing priority to blocker since without this Palm Sync cannot be used.

Comment 5

[Sean Su]

Comment on attachment 102528 [details] [diff] [review]
patch to take care of all 3 issues above

r=ssu

Comment 6

[Rajiv Dayal]

Hi Dan,

Can u please super review this bug, thanks.

Comment 7

[Daniel Veditz [:dveditz]]

Comment on attachment 102528 [details] [diff] [review]
patch to take care of all 3 issues above

>     // Construct the path of the Palm Desktop Conduit Manager
>     TCHAR   szPDCondMgrPath[_MAX_PATH];
>+    strncpy(szPDCondMgrPath, szPalmDesktopDirectory, _MAX_PATH);

In general it's better to use sizeof(<buffer>) when possible to avoid changing
one definition and forgetting to catch the others. Note that if
szPalmDesktopDirectory was somehow larger than the _MAX_PATH value you pass in
the resulting string would not be null terminated.

>     strncat(szPDCondMgrPath, DIRECTORY_SEPARATOR_STR, _MAX_PATH-strlen(szPDCondMgrPath));
>     strncat(szPDCondMgrPath, CONDMGR_FILENAME, _MAX_PATH-strlen(szPDCondMgrPath));

Similar issues here. Maybe you could do _snprintf("%s%s%s",_MAX_PATH, ...)
instead. Check for a negative return value to see if you overflowed, or just
always slap a null in the last character of the buffer.

>+    if( (*hCondMgrDll=LoadLibrary(szPDCondMgrPath)) != NULL )
>+        // Successfully loaded CondMgr Library from Palm Desktop Directory
>     return 0;
>+
>+    return IDS_ERR_LOADING_CONDMGR;

The indentation here is confusing. I assume return 0; should be indented?

>+    if( (hMozPalmSyncProxyDll=LoadLibrary("PalmSyncProxy.dll")) != NULL ) {

Could you use a full path here? It's a pretty generic name, I'd hate to pick up
the wrong one from the windows path if ours wasn't installed for some reason.

Comment 8

[Rajiv Dayal]

Attachment: updated patch to take care of sr comments

Hi Dan, Can you please sr this patch, thanks.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 102631 [details] [diff] [review]
updated patch to take care of sr comments

sr=dveditz

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 102631 [details] [diff] [review]
updated patch to take care of sr comments

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 11

[Rajiv Dayal]

fix has already landed, marking as fixed.

Comment 12

[K'Trina Medina]

Verified code fix. Rajiv, I'm assuming it's working as desired. ???

Comment 13

[Daniel Veditz [:dveditz]]

Why is this "Verified code fix"? this should be testable by anyone able to test
the Palm feature in the first place.

Comment 14

[K'Trina Medina]

Well, then can someone with a Palm give this a go?