|
Description
Marcus Brinkmann
2010-03-16 09:21:19 UTC
Created attachment 34111 [details] [review] Convert windows port to use Windows API more consistently, in preparation of WinCE port. Created attachment 34112 [details] [review] Make dependency on signal.h and locale.h optional Created attachment 34113 [details] [review] Detect WinCE in configure Created attachment 34114 [details] [review] Support WinCE throughout the code base. Created attachment 34115 [details] [review] Work around CreateProcess discrepancy. Created attachment 34116 [details] [review] Port dbus-monitor to WinCE Created attachment 34117 [details] [review] Add README.wince Created attachment 34118 [details] [review] Include a patched version of libtool for WinCE Created attachment 34123 [details] [review] Port dbus launch to windows ce. Comment on attachment 34112 [details] [review] Make dependency on signal.h and locale.h optional This patch looks fine to me. Comment on attachment 34116 [details] [review] Port dbus-monitor to WinCE Looks fine. Comment on attachment 34123 [details] [review] Port dbus launch to windows ce. >+#ifdef DBUS_WINCE >+#define _mbsrchr strrchr Kind of eww... >+/* Return a malloced string encoded in UTF-8 from the wide char input >+ string STRING. Caller must free this value. Returns NULL on >+ failure. Caller may use GetLastError to get the actual error >+ number. The result of calling this function with STRING set to >+ NULL is not defined. */ >+static char * >+wchar_to_utf8 (const wchar_t *string) We should probably preemptively put these in say dbus-sysdeps-util-win32.c. The rest of the changes are a bit hard for me to evaluate but don't seem offhand crazy. Could be a bit cleaner of course but I wouldn't block you on it. Comment on attachment 34118 [details] [review] Include a patched version of libtool for WinCE This needs to be used *ONLY* if we're building on wince. You could call it libtool.wince and reference it in the README file you made. Or it needs to just live on the machine of whoever builds on Windows CE for now. Comment on attachment 34113 [details] [review] Detect WinCE in configure Looks ok. Comment on attachment 34111 [details] [review] Convert windows port to use Windows API more consistently, in preparation of WinCE port. Scanned through quickly, looks good. Comment on attachment 34123 [details] [review] Port dbus launch to windows ce. Putting sysdeps stuff for tools/ into its own file is a good idea. If you want, I can refactor the dbus-monitor and dbus-launch-win patches and submit new ones, or I can send you patches for that refactoring on top of those. It's not a problem for me to allocate time to improve the patches to be cleaner. If we can go forward with them anyway and make them cleaner down the road, even better (less ping-ponging with git rebase -i for me :). I have my own idea of which parts of the port are not super-clean yet, but if there are items beside those you mentioned that you would like to see cleaned up, please tell me so I can allocate the time for it. Comment on attachment 34118 [details] [review] Include a patched version of libtool for WinCE As your policy seems to be that no autoconf-auto-installed files are part of the build, it would be better to just not apply this patch. I will submit a patch for the README.wince file that explains the situation, provides a patch for a recent libtool version, and explains which version of libtool meets the requirement. That will do fine for now! (In reply to comment #1) > Created an attachment (id=34111) [details] > Convert windows port to use Windows API more consistently, in preparation of > WinCE port. > I see one issue with dbus_file_read: there are no functions for opening and closing files on the same level as dbus_file_read By introducing dbus_file_open and dbus_file_close in which the related native api calls are encapsulated, code will be more readable and easier to maintain. (In reply to comment #18) > I see one issue with dbus_file_read: > > there are no functions for opening and closing files on the same level as > dbus_file_read > > By introducing dbus_file_open and dbus_file_close in which the related native > api calls are encapsulated, code will be more readable and easier to maintain. If a generic file API is desired, then yes, it should have more functions, but it also needs to abstract a file descriptor/handle. The existing _dbus_file_read did not do that, so I removed its public declaration (which was not used or exported anyway) and made the function static (and moved it up in the file). I didn't introduce it, though. (In reply to comment #12) > (From update of attachment 34123 [details] [review]) > > >+#ifdef DBUS_WINCE > >+#define _mbsrchr strrchr > > Kind of eww... > > >+/* Return a malloced string encoded in UTF-8 from the wide char input > >+ string STRING. Caller must free this value. Returns NULL on > >+ failure. Caller may use GetLastError to get the actual error > >+ number. The result of calling this function with STRING set to > >+ NULL is not defined. */ > >+static char * > >+wchar_to_utf8 (const wchar_t *string) > > We should probably preemptively put these in say dbus-sysdeps-util-win32.c. > > The rest of the changes are a bit hard for me to evaluate but don't seem > offhand crazy. Could be a bit cleaner of course but I wouldn't block you on > it. > Instead of emulating the ...A functions in dbus-launch-win.c would not make using the ...W functions by default the code much cleaner and much of the emulation functions obsolate ? (In reply to comment #20) > (In reply to comment #12) > > (From update of attachment 34123 [details] [review] [details]) > Instead of emulating the ...A functions in dbus-launch-win.c would not make > using the ...W functions by default the code much cleaner and much of the > emulation functions obsolate ? I don't know, but probably not. Let's look at the options. Right now, the conversion happens at the Windows API level, which is identical to what was already happening (internally, Windows FooA functions convert and dispatch to FooW). The conversion is isolated to 17 functions in a single file (dbus-sysdeps-wince-glue) and contained in functions which only do conversions and nothing else. If you bump it up one layer, you end up with conversion back and forth in all string-processing sysdeps functions. Those functions already do interesting stuff, and adding conversions to them does not seem appealing. The ones using DBusString are probably easy to convert by extending the DBusString class, but the ones using const char * will get more messy. There are probably more of those than 17. I didn't want to post it just yet. Gr. (In reply to comment #21) > (In reply to comment #20) > > (In reply to comment #12) > > > (From update of attachment 34123 [details] [review] [details] [details]) > > Instead of emulating the ...A functions in dbus-launch-win.c would not make > > using the ...W functions by default the code much cleaner and much of the > > emulation functions obsolate ? > > I don't know, but probably not. Let's look at the options. Right now, the > conversion happens at the Windows API level, which is identical to what was > already happening (internally, Windows FooA functions convert and dispatch to > FooW). The conversion is isolated to 17 functions in a single file > (dbus-sysdeps-wince-glue) and contained in functions which only do conversions > and nothing else. > > If you bump it up one layer, you end up with conversion back and forth in all > string-processing sysdeps functions. Those functions already do interesting > stuff, and adding conversions to them does not seem appealing. The ones using > DBusString are probably easy to convert by extending the DBusString class, but > the ones using const char * will get more messy. There are probably more of > those than 17. Continueing, if you cut at the public library API, you have to change all of the libdbus code to use string literals L"" instead of "", affecting non-windows targets, which seems highly undesirable. It is true that taking these larger approaches would get rid of 17 wrapper functions in the dbus-sysdeps-wince-glue.c file. However, having UNICODE in these places provides no real benefit, as there are almost no strings processed that are expected to be non-ASCII. The main exception are filenames, but even those are mostly ASCII on WinCE, certainly in the paths that are of concern here (installation paths, configuration files etc). The only function that is not multibyte safe in this port is the one stripping of the last filename component from a full absolute filename, and the reason is that the simple approach (converting utf to utf16, stripping off the component, converting back and checking if the result is a prefix of the original string) didn't seem particularly safe either to me (in the presence of non-canonical forms). The main reason for doing it in its own file for Windows CE only is to reduce the impact of the port on the other ports, including Windows. Even if it is decided that the Windows (and Windows CE) port should use the UNICODE interface as broadly and early as possible, I would suggest to make that a separate project. I am willing to help with that, if that's what you want. (In reply to comment #12) > >+/* Return a malloced string encoded in UTF-8 from the wide char input > >+ string STRING. Caller must free this value. Returns NULL on > >+ failure. Caller may use GetLastError to get the actual error > >+ number. The result of calling this function with STRING set to > >+ NULL is not defined. */ > >+static char * > >+wchar_to_utf8 (const wchar_t *string) > > We should probably preemptively put these in say dbus-sysdeps-util-win32.c. > > The rest of the changes are a bit hard for me to evaluate but don't seem > offhand crazy. Could be a bit cleaner of course but I wouldn't block you on > it. Colin, I said before that's a good idea, but now I am thinking you may have meant something different from what I thought you meant. There are _dbus_win_utf8_to_utf16 and _dbus_win_utf16_to_utf8 functions which do the same job as utf8_to_wchar and wchar_to_utf8, and they work just fine, but they are not available in the tools/ directory, as they are part of the internal static version of libdbus. I think I will rewrite dbus-launch-win.c to use the unicode interfaces on Windows and Windows CE, which will get rid of this code duplication. Created attachment 34241 [details] [review] UNICODE version of dbus-launch-win.c This is the unicodified version of dbus-launch-win.c The following patch is moved to here from bug 27194 diff --git a/dbus/dbus-sysdeps-wince-glue.c b/dbus/dbus-sysdeps-wince-glue.c index 38b59c6..204e719 100644 --- a/dbus/dbus-sysdeps-wince-glue.c +++ b/dbus/dbus-sysdeps-wince-glue.c @@ -26,6 +26,7 @@ * */ +#include <config.h> #include "dbus-internals.h" #include "dbus-sysdeps.h" #include "dbus-sysdeps-win.h" (In reply to comment #24) > Created an attachment (id=34241) [details] > UNICODE version of dbus-launch-win.c > > This is the unicodified version of dbus-launch-win.c > I tried this msvc++ 2008 express edition and have been forced to use the local implementation of wcscat_s. Appended is the required patch diff --git a/tools/dbus-launch-win.c b/tools/dbus-launch-win.c index a192b8a..62dc743 100644 --- a/tools/dbus-launch-win.c +++ b/tools/dbus-launch-win.c @@ -31,11 +31,17 @@ #include <mbstring.h> #include <assert.h> -#if defined __MINGW32__ || (defined _MSC_VER && _MSC_VER <= 1310) +#if defined __MINGW32__ || (defined _MSC_VER && (_MSC_VER <= 1310 || _MSC_VER == 1500)) /* save string functions version */ #define errno_t int +#if _MSC_VER == 1500 +/* wcscat_s does not work in vs 2008 express*/ +#define wcscat_s _wcscat_s +#define wcscpy_s _wcscpy_s +#endif + static errno_t wcscat_s (wchar_t *dest, size_t size, wchar_t *src) { (In reply to comment #26) > (In reply to comment #24) > I tried this msvc++ 2008 express edition and have been forced to use the local > implementation of wcscat_s. Appended is the required patch I don't know what's right for _MSC_VER, the <=1310 was probably true for the strcat_s/strcpy_s stuff. It may very well be <=1500 for the wcs variants, but I don't have any version of VS and the online MSDN documentation doesn't seem to give me any info on this either. (In reply to comment #24) > Created an attachment (id=34241) [details] > UNICODE version of dbus-launch-win.c > > This is the unicodified version of dbus-launch-win.c > I compiled this with msvc and mingw and got a runtime error: prompt>cd <...path> prompt>set DBUS_VERBOSE=1 prompt>dbus-launch <...path>\dbus-daemon.exe --session Could not start dbus-daemon. error=6 'invalid handle error' Directly starting dbus-daemon --session let dbus daemon run Any idea how to fix ? (In reply to comment #1) > Created an attachment (id=34111) [details] > Convert windows port to use Windows API more consistently, in preparation of > WinCE port. > Please run bus-test with this patch applied. It will break on windows "Testing valid files": basic.conf with Failed to load valid file but still had memory: Failed to read directory <build_root>\test\data\valid-config-files\basic.d\basic.d": Das System kann den angegebenen Pfad nicht finden. I guess the new implementation do not distingiush between files an dir as the old one does. This let _dbus_directory_get_next_file return a directory 'basic.d' as file which is wrong. (In reply to comment #2) > Created an attachment (id=34112) [details] > Make dependency on signal.h and locale.h optional > looks good, I committed this and a related patch for the cmake build system. Comment on attachment 34112 [details] [review] Make dependency on signal.h and locale.h optional committed (In reply to comment #29) > (In reply to comment #1) > > Created an attachment (id=34111) [details] [details] > > Convert windows port to use Windows API more consistently, in preparation of > > WinCE port. > > > Please run bus-test with this patch applied. It will break on windows "Testing > valid files": basic.conf with > > Failed to load valid file but still had memory: Failed to read directory > <build_root>\test\data\valid-config-files\basic.d\basic.d": Das System kann den > angegebenen Pfad nicht finden. Thanks for finding this. It's just a missing error code conversion: diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index 174b8a1..b3fd9c0 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -123,6 +123,7 @@ _dbus_win_error_from_last_error (void) return DBUS_ERROR_FILE_EXISTS; case ERROR_FILE_NOT_FOUND: + case ERROR_PATH_NOT_FOUND: return DBUS_ERROR_FILE_NOT_FOUND; } > I guess the new implementation do not distingiush between files an dir as the > old one does. > This let _dbus_directory_get_next_file return a directory 'basic.d' as file > which is wrong. What is actually happening: test/data/valid-config-files/basic.d/basic.conf contains the instruction: "<includedir>basic.d</includedir>" but the directory test\data\valid-config-files\basic.d\basic.d does not exist. But PATH_NOT_FOUND is not mapped to DBUS_ERROR_FILE_NOT_FOUND, so the test case does not ignore it. (In reply to comment #28) > (In reply to comment #24) > > Created an attachment (id=34241) [details] [details] > > UNICODE version of dbus-launch-win.c > > > > This is the unicodified version of dbus-launch-win.c > > > I compiled this with msvc and mingw and got a runtime error: > > prompt>cd <...path> > prompt>set DBUS_VERBOSE=1 > prompt>dbus-launch > <...path>\dbus-daemon.exe --session > Could not start dbus-daemon. error=6 'invalid handle error' > > Directly starting dbus-daemon --session let dbus daemon run > > Any idea how to fix ? > Ah ok. The actual error is not 6, but 87: The CloseHandle instructions overwrite the last error value. That should be fixed by moving CloseHandle after the error check. The actual error is the use of CREATE_NEW_CONSOLE if DBUS_VERBOSE is set(!) in conjunction with DETACHED_PROCESS, which is not allowed (they are mutually exclusive). I think that using DETACHED_PROCESS is correct (that's why I added it), but I also see a need for being able to see the output of dbus-daemon. As I am not sure what CREATE_NEW_CONSOLE and DETACHED_PROCESS *really* do (the Windows console and window code is bizarrly mysterious in this area, and poorly documented), I will try to find some wizards to get some advice (or maybe you know something). It will be easy to fix though (use one flag or the other, depending on what the desired effect is). Simply not setting DBUS_VERBOSE and/or removing either CREATE_NEW_CONSOLE and/or DETACHED_PROCESS from the patch will work for now. Stay tuned for the follow up. (In reply to comment #33) > (In reply to comment #28) > > (In reply to comment #24) > > > Created an attachment (id=34241) [details] [details] [details] > > > UNICODE version of dbus-launch-win.c > > > > > > This is the unicodified version of dbus-launch-win.c > > > > > I compiled this with msvc and mingw and got a runtime error: > > > > prompt>cd <...path> > > prompt>set DBUS_VERBOSE=1 > > prompt>dbus-launch > > <...path>\dbus-daemon.exe --session > > Could not start dbus-daemon. error=6 'invalid handle error' > > > > Directly starting dbus-daemon --session let dbus daemon run > > > > Any idea how to fix ? > > > > Ah ok. The actual error is not 6, but 87: The CloseHandle instructions > overwrite the last error value. That should be fixed by moving CloseHandle > after the error check. > > The actual error is the use of CREATE_NEW_CONSOLE if DBUS_VERBOSE is set(!) in > conjunction with DETACHED_PROCESS, which is not allowed (they are mutually > exclusive). I think that using DETACHED_PROCESS is correct (that's why I added > it), but I also see a need for being able to see the output of dbus-daemon. > > As I am not sure what CREATE_NEW_CONSOLE and DETACHED_PROCESS *really* do (the > Windows console and window code is bizarrly mysterious in this area, and poorly > documented), I will try to find some wizards to get some advice (or maybe you > know something). It will be easy to fix though (use one flag or the other, > depending on what the desired effect is). Simply not setting DBUS_VERBOSE > and/or removing either CREATE_NEW_CONSOLE and/or DETACHED_PROCESS from the > patch will work for now. Stay tuned for the follow up. > As far As i remember does CREATE_NEW_CONSOLE open a command windows for the created process, which makes it able to see verbose messages for the daemon. (In reply to comment #34) > As far As i remember does CREATE_NEW_CONSOLE open a command windows for the > created process, which makes it able to see verbose messages for the daemon. Solid information is hard to come by, but CREATE_NEW_CONSOLE creates a new console instead of inheriting the current one, while DETACHED_PROCESS creates the new process without a console, which is of course mutually exclusive. There is also the issue of a console popup (SHOW_MINIMIZE). I added DETACHED_PROCESS because it seems to me to be desirable to start dbus-daemon as a daemonized process that does not share its console with dbus-launch. I am not sure if CREATE_NEW_CONSOLE is desirable/useful in the logging/debugging case. I'll play around with the flags and see if there are any visible differences. (In reply to comment #35) > (In reply to comment #34) > > As far As i remember does CREATE_NEW_CONSOLE open a command windows for the > > created process, which makes it able to see verbose messages for the daemon. > > Solid information is hard to come by, but CREATE_NEW_CONSOLE creates a new > console instead of inheriting the current one, while DETACHED_PROCESS creates > the new process without a console, which is of course mutually exclusive. > There is also the issue of a console popup (SHOW_MINIMIZE). > > I added DETACHED_PROCESS because it seems to me to be desirable to start > dbus-daemon as a daemonized process that does not share its console with > dbus-launch. > I am not sure if CREATE_NEW_CONSOLE is desirable/useful in the > logging/debugging case. dbus logging uses stderr for debug output when DBUS_VERBOSE=1 is set. How would it be possible to see the log when dbus-launch runs the daemon in the background ? (In reply to comment #36) > > I added DETACHED_PROCESS because it seems to me to be desirable to start > > dbus-daemon as a daemonized process that does not share its console with > > dbus-launch. > > > I am not sure if CREATE_NEW_CONSOLE is desirable/useful in the > > logging/debugging case. > > dbus logging uses stderr for debug output when DBUS_VERBOSE=1 is set. How would > it be possible to see the log when dbus-launch runs the daemon in the > background ? > In the logging case, DETACHED_PROCESS should not be used. But I am not sure if CREATE_NEW_CONSOLE should be used, or just 0, instead. CREATE_NEW_CONSOLE creates a new console, while not specifying it causes dbus-daemon to inherit the console from dbus-launch. On Windows, inheriting the existing console is probably ok, on Win CE one may want to use CREATE_NEW_CONSOLE, I have to test that. (In reply to comment #37) > (In reply to comment #36) > > > > I added DETACHED_PROCESS because it seems to me to be desirable to start > > > dbus-daemon as a daemonized process that does not share its console with > > > dbus-launch. > > > > > I am not sure if CREATE_NEW_CONSOLE is desirable/useful in the > > > logging/debugging case. > > > > dbus logging uses stderr for debug output when DBUS_VERBOSE=1 is set. How would > > it be possible to see the log when dbus-launch runs the daemon in the > > background ? > > > > In the logging case, DETACHED_PROCESS should not be used. agreed > But I am not sure if > CREATE_NEW_CONSOLE should be used, or just 0, instead. CREATE_NEW_CONSOLE > creates a new console, while not specifying it causes dbus-daemon to inherit > the console from dbus-launch. yes, but inheriting parents console does not work because dbus-launch starts dbus-daemon and exists immediatly, which means there is no parent console for dbus-daemon. > On Windows, inheriting the existing console is probably ok, As mentioned above - it does not work yet, so there is only the CREATE_NEW_CONSOLE option. Or do you see a way to redirect dbus-launch's parent console to CreateProcess ? > on Win CE one may want to use CREATE_NEW_CONSOLE, I have to test that. (In reply to comment #38) > yes, but inheriting parents console does not work because dbus-launch starts > dbus-daemon and exists immediatly, which means there is no parent console for > dbus-daemon. Right. So it should be DETACHED_PROCESS if !verbose and CREATE_NEW_CONSOLE if verbose. I will test this combination. Created attachment 34933 [details] [review] Convert windows port to use Windows API more consistently, in preparation of WinCE port. (REVISION 2) Created attachment 34934 [details] [review] Detect WinCE in configure Created attachment 34935 [details] [review] Support WinCE throughout the code base. Created attachment 34936 [details] [review] Work around CreateProcess discrepancy. Created attachment 34937 [details] [review] Port dbus tools to windows ce. Created attachment 34938 [details] [review] Add README.wince Created attachment 34939 [details] [review] Add/Fix WinCE support in cmake files. Created attachment 34940 [details] [review] Replace strtoll/strtoull for Windows CE. VARIATION 1/2 Created attachment 34941 [details] [review] Replace strtoll/strtoull for Windows CE. VARIATION 2/2 Created attachment 34942 [details] [review] Custom version of libtool for convenience of potential testers, DO NOT APPLY. I updated the series of patches, incorporating the input from the discussion and testing. If anything is missing for inclusion into mainline (more testing, etc), please let me know if I can be of any assistance. Changes to first round of patches: 0001-Prepare-for-WinCE-port-Convert-windows-code-to-nativ.patch: This now includes a port of dbus-pipe-win.c to the native Windows API (contributed by Romain). Also includes the error code mapping fix from the comment #32. 0002-Add-WinCE-detection-to-configure.in-and-choose-right.patch: Unchanged. 0003-Add-support-for-Windows-CE-to-the-code-base.patch: Includes changes needed for Microsoft SDK (by Romain), which differs from cegcc in minor ways. 0004-Windows-CE-has-a-different-understanding-of-command-.patch: Unchanged. 0005-Port-dbus-tools-to-Windows-CE.patch: Contains all patches related to the tools directory, and a version of dbus-launch-win.c that accumulates the discussion on spawn flags in the comments. 0006-Add-README-for-Windows-CE.patch: Contains a reference to the libtool requirements for Windows CE. 0007-update-WinCE-cmake-support.patch: Adjustments to WinCE building with cmake under Windows (using Microsoft SDK) by Romain. 0008-Add-replacement-functions-strtoll-strtoull-for-dbus-.patch, 0009-Simple-fix-for-Windows-CE-s-lack-of-strtoll.patch: Two patches (alternatives) for the same problem: Windows CE does not have strtoll and strtoull, needed by dbus-send. The first patch provides replacement implementations taken from BSD, the second patch cripples dbus-send and is thus much simpler (just an ifdef). You decide (we feel 64 bit args for dbus-send is not important to have on windows ce but provided a full patch for correctness anyway). 0010-Include-a-patched-version-of-libtool-good-for-Window.patch: Just provided for easy access to a working libtool, for reference only. Do not apply. Created attachment 34943 [details] [review] Support WinCE throughout the code base. The previous submission contained an embarrassing typo. With regards to the strtoll/strtoull patches, please note that these have not been part of the previous revision of the patches as they are only needed with the Microsoft SDK: cegcc provides them. Comment on attachment 34939 [details] [review] Add/Fix WinCE support in cmake files. committed Created attachment 34967 [details] [review] Add missing HAVE_ macros to cmake Patch by Romain to bring the cmake build up to the other patches. This includes tests, among others, for strtoll/strtoull, which would be needed for the 34940 patch (VARIATION 1/2), and would not do much harm (but are not required) for the second variation. Comment on attachment 34933 [details] [review] Convert windows port to use Windows API more consistently, in preparation of WinCE port. (REVISION 2) committed Comment on attachment 34933 [details] [review] Convert windows port to use Windows API more consistently, in preparation of WinCE port. (REVISION 2) no problems on win32 detected committed Comment on attachment 34934 [details] [review] Detect WinCE in configure committed Comment on attachment 34936 [details] [review] Work around CreateProcess discrepancy. committed Comment on attachment 34936 [details] [review] Work around CreateProcess discrepancy. committed Comment on attachment 34967 [details] [review] Add missing HAVE_ macros to cmake committed Comment on attachment 34943 [details] [review] Support WinCE throughout the code base. committed. You may add a copyright note to dbus-sysdeps-wince-glue.* Comment on attachment 34937 [details] [review] Port dbus tools to windows ce. committed Comment on attachment 34940 [details] [review] Replace strtoll/strtoull for Windows CE. VARIATION 1/2 committed Comment on attachment 34938 [details] [review] Add README.wince committed Review of attachment 34933 [details] [review]: On win32 there are warnings: D:\daten\kde\download\svn-src\dbus-freedesktop-src-ssh\dbus\dbus-file-win.c(71) : warning C4102: 'again': Unreferenzierte Marke D:\daten\kde\download\svn-src\dbus-freedesktop-src-ssh\dbus\dbus-file-win.c(170) : warning C4018: '<': Konflikt zwischen 'signed' und 'unsigned' Comment on attachment 34941 [details] [review] Replace strtoll/strtoull for Windows CE. VARIATION 2/2 Obsolete because the other variant was applied. Created attachment 34982 [details] [review] Include strtoll.c and strtoull.c in distribution This patch adds two missing files to the distribution with autoconf. They are not added automagically because they are .c files that are #include'd. A bit lame, but as we have more than one built system to support, this seems the simplest solution. There may be a corresponding change necessary in the cmake files. Created attachment 34983 [details] [review] Remove obsolete files. Since we use smart exports now, the def.in and def.pre files are no longer used. Created attachment 34984 [details] [review] Fix warnings on windows builds This fixes the remaining warnings on Windows under autoconf builds (except for an annoying -ffunction-sections warning that may be a bug in gcc). Created attachment 34985 [details] [review] Fix warnings on windows ce builds Windows CE had a couple of warnings that are fixed with this patch. There are also the same bogus(?) -ffunction-sections warnings as with windows. Also, there are many bogus(?) alignment warnings on windows ce builds with autoconf. I checked some of them and they were all false warnings (a proper alignment macro was used by dbus in the source line before the warning). Not sure how to fix those. Ralf, thanks a lot for applying the previous round of patches, it's a great relief to be able to work off master instead of a custom branch. Now the energy can go into clean up, more testing and keeping everything up to date. As a first step, I just added 4 patches that clean up a bunch of minor issues. I have another issue on my list I plan to work on soon: Testing the various combinations with/without maintainer mode, with/without tests etc. I think that there are still some issues with symbol exports in some of these at least (at least for autoconf based builds). A bigger open issue is porting the test suite to Windows CE. I won't have time for this in April, but it's in our backlog, so we'll see. Comment on attachment 34985 [details] [review] Fix warnings on windows ce builds committed Comment on attachment 34982 [details] [review] Include strtoll.c and strtoull.c in distribution committed Comment on attachment 34983 [details] [review] Remove obsolete files. committed Comment on attachment 34942 [details] [review] Custom version of libtool for convenience of potential testers, DO NOT APPLY. committed Comment on attachment 34984 [details] [review] Fix warnings on windows builds committed (In reply to comment #71) > Ralf, thanks a lot for applying the previous round of patches, it's a great > relief to be able to work off master instead of a custom branch. Now the > energy can go into clean up, more testing and keeping everything up to date. > > As a first step, I just added 4 patches that clean up a bunch of minor issues. Applying Fix warnings on Windows CE target. .dotest/patch:73: trailing whitespace. #define _WIN32_WCE 0x0401 warning: 1 line adds whitespace errors. > > I have another issue on my list I plan to work on soon: Testing the various > combinations with/without maintainer mode, with/without tests etc. I think > that there are still some issues with symbol exports in some of these at least > (at least for autoconf based builds). okay > A bigger open issue is porting the test suite to Windows CE. I won't have time > for this in April, but it's in our backlog, so we'll see. I suggest to check before, that there are no issues on win32, otherwise you may fight on areas which has to be solved on a deeper level. I spent some time on the testsuite to analyse where it works and where not, fixed already some issues and will report the remaining to the list probably next week. (In reply to comment #75) > (From update of attachment 34942 [details] [review]) > committed Colin vetoed this patch (he wants to keep libtool out of the repository). OTOH, I am not sure you committed it, because when I pull from the repo, I still get the same version as last night. (In reply to comment #78) > (In reply to comment #75) > > (From update of attachment 34942 [details] [review] [details]) > > committed > > Colin vetoed this patch (he wants to keep libtool out of the repository). > > OTOH, I am not sure you committed it, because when I pull from the repo, I > still get the same version as last night. no, -> restored obsolate flag (In reply to comment #72) > (From update of attachment 34985 [details] [review]) > committed Ralf, for this and the other three patches I recently submitted, I can't find them in the dbus master git repository. Is there another branch I should look at? I've seen in the git history that you seem to merge from another branch at times, but I don't know if that's the case here. Thanks, Marcus (In reply to comment #80) > (In reply to comment #72) > > (From update of attachment 34985 [details] [review] [details]) > > committed > > Ralf, for this and the other three patches I recently submitted, I can't find > them in the dbus master git repository. Is there another branch I should look > at? no > I've seen in the git history that you seem to merge from another branch at > times, but I don't know if that's the case here. > > Thanks, > Marcus git push did it :-) Created attachment 35206 [details] [review] Move definition of _WIN32_WCE to configure.in This moves _WIN32_WCE = 0x502 to configure.in. Romain pointed out that cmake already defines it. This is just as well or even better than defining it only in a single header file. Created attachment 35390 [details] [review] Fix GetModuleFileNameA and RegQueryValueExA There were two bugs: Some memory leaks in RegQueryValueExA both in error and normal case, and a too restrict buffer limit in GetModuleFileNameA. Created attachment 35432 [details] [review] Fix infinite recursion on Windows CE. A recent change in dbus memory allocation (adding a dbus_verbose call) causes an infinite recursion on Windows CE, because getenv for DBUS_VERBOSE does call the dbus memory allocation routines. This patch special cases this code path so the infinite recursion is avoided. Created attachment 35475 [details] [review] Define _WIN32_WCE for autoconf based builds. This is necessary to get the getaddrinfo prototype in autoconf based builds (cmake already defines it). Comment on attachment 35432 [details] [review] Fix infinite recursion on Windows CE. This patch seem to be a duplicate of https://bugs.freedesktop.org/attachment.cgi?id=35390 Comment on attachment 35475 [details] [review] Define _WIN32_WCE for autoconf based builds. committed Created attachment 35485 [details] [review] Fix infinite recursion on Windows CE. Correct version. Review of attachment 35485 [details] [review]: get_verbose_setting() returns a string created with malloc. Do you take care of freeing this memory anywhere ? Ralf, see the (In reply to comment #89) > Review of attachment 35485 [details] [review]: > > get_verbose_setting() returns a string created with malloc. Do you take care of > freeing this memory anywhere ? Sure, see the static char pointer "past_result" in our getenv() substitute (line 287ff). Comment on attachment 35390 [details] [review] Fix GetModuleFileNameA and RegQueryValueExA committed Comment on attachment 35485 [details] [review] Fix infinite recursion on Windows CE. committed Created attachment 35593 [details] [review] Fix _IOLBF for Windows CE and also define _IONBF This fixes a wrong constant for MSVC based builds, and also fixes a failure-to-build-from-source due to c38809e8da115ec9bf2b46837a88557e1607d5a6 (2010-05-12) Comment on attachment 35593 [details] [review] Fix _IOLBF for Windows CE and also define _IONBF committed Comment on attachment 35593 [details] [review] Fix _IOLBF for Windows CE and also define _IONBF committed Concerning the use of *W api. I would actually militate in favour of using UTF-8 internally everywhere in dbus and of converting the corresponding strings to UTF-16 when we use them in the Win32 api calls. This is something glib uses in the Gnome world and it works pretty well. And as I don't expect us ever supporting unmaintained windows 9x and Me, it would be really a nice thing for people that name their folders in Thai, Arabic, or Traditional Chinese. (In reply to comment #96) > Concerning the use of *W api. I would actually militate in favour of using > UTF-8 internally everywhere in dbus and of converting the corresponding strings > to UTF-16 when we use them in the Win32 api calls. > This is something glib uses in the Gnome world and it works pretty well. And as > I don't expect us ever supporting unmaintained windows 9x and Me, it would be > really a nice thing for people that name their folders in Thai, Arabic, or > Traditional Chinese. You are right that the FooA ANSI interfaces do not actually support UTF-8 but expect some native encoding (for example for Chinese they expect gb2312). This is a limitation only if you expect the dbus install and service directories to be non-ASCII. I am not sure that is the case. What's the program directory called on russian and chinese Windows? If there is a desire or need to support non-ASCII filenames, then the way you said it is the way to do it. Of course, this would greatly simplify the Windows CE port. |
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.