Bug 41558 - On Windows, include dbus-1.dll path into search path when autolaunching dbus-daemon.exe
Summary: On Windows, include dbus-1.dll path into search path when autolaunching dbus-...
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: All Windows (All)
: medium enhancement
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-07 07:25 UTC by Jesper Alf Dam
Modified: 2011-11-08 04:41 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Add dbus1.dll dir to search path (1.54 KB, application/octet-stream)
2011-10-07 07:25 UTC, Jesper Alf Dam
Details
fixed patch (1.88 KB, patch)
2011-10-28 04:07 UTC, Jesper Alf Dam
Details | Splinter Review
Minimal test case (695 bytes, text/plain)
2011-10-28 04:10 UTC, Jesper Alf Dam
Details
updated to use _dbus_win_get_dll_hmodule (1.62 KB, patch)
2011-10-28 06:59 UTC, Jesper Alf Dam
Details | Splinter Review

Description Jesper Alf Dam 2011-10-07 07:25:01 UTC
Created attachment 52085 [details]
Add dbus1.dll dir to search path

Attached patch adds the directory that dbus-1.dll is found in, to the search path when autolaunch: tries to locate and start the dbus-daemon executable.

Rationale: on typical Windows systems, the dbus daemon is likely to be found in the same dir as dbus-1.dll, and this dir is unlikely to be in the users PATH. Adding this dir to the search path when launching the daemon avoids having to rely on the PATH environment variable in the common case.

I'm pretty new to dbus, and this is my first patch submission here, so let me know if anything should be done differently. :)
Comment 1 Ralf Habacker 2011-10-18 09:12:05 UTC
Hi Jesper, 

many thanks for you contribution. 
I tried to apply this patch to dbus git master branch and got errors: 

C:\kde\trunk\git\dbus-src-git>git apply search-patch.patch
search-patch.patch:13: trailing whitespace.
      // Look in directory containing dbus shared library
search-patch.patch:14: trailing whitespace.
      HMODULE hmod = 0;
search-patch.patch:15: trailing whitespace.
      if (GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS |
search-patch.patch:16: trailing whitespace.
                             GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
search-patch.patch:17: trailing whitespace.
                             (LPCSTR)_dbus_get_autolaunch_address, &hmod))
fatal: corrupt patch at line 38

BTW: Please provide patches in the future with "git format-patch -o <outdir> origin", because they are "ready to commit".  

The patch itself looks good, I'm currently thinking how to find a test case. 

On Windows 7 I did the following: 
1. I installed dbus in c:\dbus. 
2. I do not have c:\dbus\bin in the system path 
3. I entered c:\
4. I run c:\dbus\bin\dbus-monitor.exe 

The dbus-daemon is autolaunched, which means the recent SearchPath implementation found him. 

Any ideas ? 

Regards 
Ralf
Comment 2 Jesper Alf Dam 2011-10-28 04:07:44 UTC
Created attachment 52848 [details] [review]
fixed patch
Comment 3 Jesper Alf Dam 2011-10-28 04:10:52 UTC
Created attachment 52849 [details]
Minimal test case

The attached program is the smallest I could come up with which reproduces the problem.

If the dbus dll and the daemon are both in a dir that is not part of the default search path, and dbus-1.dll is dynamically loaded with LoadLibrary(), it will fail to locate and launch the daemon without this patch.
Comment 4 Ralf Habacker 2011-10-28 06:02:10 UTC
Comment on attachment 52848 [details] [review]
fixed patch

Review of attachment 52848 [details] [review]:
-----------------------------------------------------------------

+      // Look in directory containing dbus shared library
+      HMODULE hmod = 0;
+      if (GetModuleHandleExA(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | 
+                             GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+                             (LPCSTR)_dbus_get_autolaunch_address, &hmod))
+        {

To get a handle to the dbus-1.dll there is alreay a function named dbus_win_get_dll_hmodule() available. You should use that.
Comment 5 Jesper Alf Dam 2011-10-28 06:59:45 UTC
Created attachment 52857 [details] [review]
updated to use _dbus_win_get_dll_hmodule

Thanks for the tip. Updated the patch to use that instead. :)
Comment 6 Ralf Habacker 2011-10-28 12:50:51 UTC
(In reply to comment #5)
> Created attachment 52857 [details] [review] [review]
> updated to use _dbus_win_get_dll_hmodule
> 
> Thanks for the tip. Updated the patch to use that instead. :)

Patch works as expected and has been committed. 
BTW: As an example I added some optimations for error handling and log printing. 

More optimations are possible for example "HMODULE hmod" could be completly skipped by placing _dbus_win_get_dll_hmodule into the GetModuleFileNameA call.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.