May 2007 - Wine-Devel - list.winehq.org

Re: Adds NET START to net.exe
by Dmitry Timoshkov May 10, 2007

May 10, 2007

"Tim Schwartz" <tim(a)sanityinternet.com> wrote: > + GetServiceDisplayName(SCManager, service_name, NULL, &buffer_size); > + if(!buffer_size) You need to check the GetServiceDisplayName retrun value, not buffer_size. If GetServiceDisplayName succeeds buffer_size contains valid length. Also please numerate your retries, like "Take 3" or something like that. -- Dmitry.

1 0

[3 and 4/4] Some console improvements: console codepages - please, comment.
by Kirill K. Smirnov May 10, 2007

May 10, 2007

Hi, In my serie of four patches the two has not been applied: [kernel32/tests: Add tests for console codepages] http://www.winehq.org/pipermail/wine-patches/2007-May/039092.html [move codepages to server side] http://www.winehq.org/pipermail/wine-patches/2007-May/039093.html Please, comment, what I did wrong? Did I miss something important? Thanks -- Kirill

2 1

Re: [5/5] widl: Remove var_t's ptr_level field and start write_pointers
by Alexandre Julliard May 10, 2007

May 10, 2007

Dan Hipschman <dsh(a)linux.ucla.edu> writes: > Well, I sent this quite a while ago and it hasn't shown up on the list yet. > It's probably too big, so here's a link to the patch instead of inlining it. Please don't do that. You can gzip the patch if necessary, but please send it by mail. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 02/11] [Kernel32]: ActCtx: Added infrastructure for loading a manifest (app,
by Alexandre Julliard May 10, 2007

May 10, 2007

Eric Pouech <eric.pouech(a)wanadoo.fr> writes: > + hFile = CreateFileW(name, GENERIC_READ, 0, NULL, OPEN_EXISTING, 0, 0); > + if (hFile != INVALID_HANDLE_VALUE) > + { > + hMap = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL); > + if (hMap != INVALID_HANDLE_VALUE) > + { CreateFileMapping returns 0 on error. > + base = MapViewOfFile(hMap, FILE_MAP_READ, 0, 0, 0); > + if (base) > + { > + HANDLE hModule = (HMODULE)(ULONG_PTR)base; > + > + if (RtlImageNtHeader(hModule)) /* we got a PE file */ > + { > + if (first) > + { > + LPWSTR manifestfile; > + > + manifestfile = HeapAlloc(GetProcessHeap(), 0, > + lstrlenW(name) * sizeof(WCHAR) + sizeof(dotManifestW)); > + if (manifestfile) > + { > + strcpyW(manifestfile, name); > + strcatW(manifestfile, dotManifestW); > + ret = get_manifest_in_file(acl, ai, manifestfile, > + resname, FALSE); > + HeapFree(GetProcessHeap(), 0, manifestfile); > + } > + } > + if (ret) ret = get_manifest_in_module(acl, ai, hModule, resname); You cannot pretend that a file mapping is a module and call resource functions on it. You have to use LOAD_LIBRARY_AS_DATAFILE for that. I don't think this patch series is quite ready to go in; this is the third resend and all patches still have obvious bugs. You need to spend more time on it, this is kernel stuff, it needs to be correct. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 01/11] [Kernel32]: ActCtx: added basic structures for storing assembly information
by Alexandre Julliard May 10, 2007

May 10, 2007

Eric Pouech <eric.pouech(a)wanadoo.fr> writes: > + if (actctx->num_assemblies == actctx->allocated_assemblies) > + { > + if (actctx->allocated_assemblies) > + { > + actctx->allocated_assemblies *= 2; > + actctx->assemblies = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + actctx->assemblies, > + actctx->allocated_assemblies * sizeof(struct assembly)); > + } > + else > + { > + actctx->allocated_assemblies = 1; > + actctx->assemblies = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct assembly)); > + } > + } > + if (!actctx->assemblies) return NULL; You will corrupt the previous state if the realloc fails. This is kernel stuff, please try to be rigorous about error handling. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: kernel32: MultiByteToWideChar: MB_USEGLYPHCHARS & incorrect GetLastError() codes fix
by Kirill K. Smirnov May 10, 2007

May 10, 2007

+static void test_MultiByteToWideChar(void) +{ + int i, ret; + static char src[32]; + static WCHAR dst[32]; + static const WCHAR glyphed_dst[32] = { + 0x0000, 0x263A, 0x263B, 0x2665, 0x2666, 0x2663, 0x2660, 0x2022, + 0x25D8, 0x25CB, 0x25D9, 0x2642, 0x2640, 0x266A, 0x266B, 0x263C, + 0x25BA, 0x25C4, 0x2195, 0x203C, 0x00B6, 0x00A7, 0x25AC, 0x21A8, + 0x2191, 0x2193, 0x2192, 0x2190, 0x221F, 0x2194, 0x25B2, 0x25BC + }; + for (i=0;i<32;i++) src[i]=i; + memset(&dst, 0, sizeof(dst)); Char 0x7F should be tested too. + trace("originaldst: %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x\n", + dst[0],dst[1],dst[2],dst[3],dst[4],dst[5],dst[6],dst[7],dst[8],dst[9],dst[10],dst[11],dst[12],dst[13],dst[14],dst[15],dst[16], + dst[17],dst[18],dst[19],dst[20],dst[21],dst[22],dst[23],dst[24],dst[25],dst[26],dst[27],dst[28],dst[29],dst[30],dst[31]); + trace("glyphed_dst: %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x %.4x\n", + glyphed_dst[0],glyphed_dst[1],glyphed_dst[2],glyphed_dst[3],glyphed_dst[4],glyphed_dst[5],glyphed_dst[6],glyphed_dst[7],glyphed_dst[8],glyphed_dst[9],glyphed_dst[10],glyphed_dst[11],glyphed_dst[12],glyphed_dst[13],glyphed_dst[14],glyphed_dst[15],glyphed_dst[16], + glyphed_dst[17],glyphed_dst[18],glyphed_dst[19],glyphed_dst[20],glyphed_dst[21],glyphed_dst[22],glyphed_dst[23],glyphed_dst[24],glyphed_dst[25],glyphed_dst[26],glyphed_dst[27],glyphed_dst[28],glyphed_dst[29],glyphed_dst[30],glyphed_dst[31]); + } +} My notice may look like nitpicking, but these lines are too long.. Is it really necessary to dump entire array instead of just printing error message? -- Kirill

1 0

Re: [PATCH 2/3] winex11: Implement TINN algorithm. (try 3)
by Dmitry Timoshkov May 10, 2007

May 10, 2007

"Vitaly Budovski" <vbudovski(a)gmail.com> wrote: > +struct tinn > +{ > + /* Pointer to an element in the list. */ > + const RGBQUAD *data; > + /* Distance from the reference element. */ > + float distance; > +}; Vitaly, why do you keep resending the same patch which uses float values internally? Again, float neither doesn't allow you to process larger values, nor is acceptable for speed or adds more flexibility. -- Dmitry.

4 5

Re: kernel32: MultiByteToWideChar: MB_USEGLYPHCHARS & incorrectGetLastError() codes fix
by Dmitry Timoshkov May 10, 2007

May 10, 2007

"Andrew O. Shadoura" <bugzilla(a)tut.by> wrote: > if (!src || (!dst && dstlen)) > { > @@ -1774,18 +1773,12 @@ INT WINAPI MultiByteToWideChar( UINT page, DWORD flags, LPCSTR src, INT srclen, > > if (srclen < 0) srclen = strlen(src) + 1; > > - if (!once && (flags & MB_USEGLYPHCHARS)) > - { > - once = 1; > - FIXME("MB_USEGLYPHCHARS not supported\n"); > - } > - > switch(page) > { > case CP_SYMBOL: > - if( flags) > + if(flags) > { If you decided to fix the formatting it would be better to send it as a separate patch, and follow the style of 'if' statements above. > case CP_UTF8: > + if (flags & (~MB_ERR_INVALID_CHARS)) > + { > + SetLastError( ERROR_INVALID_FLAGS ); > + return 0; > + } Mixed tabs/spaces. > --- a/dlls/kernel32/tests/locale.c > +++ b/dlls/kernel32/tests/locale.c > @@ -2255,6 +2255,42 @@ static void test_GetCPInfo(void) > ok(cpinfo.MaxCharSize == 4, "expected 5, got 0x%x\n", cpinfo.MaxCharSize); > } > > +static void test_MultiByteToWideChar(void) This test belongs to dlls/kernel32/tests/codepage.c. > + else > + res = mbstowcs_sbcs_decompose( &table->sbcs, src, srclen, dst, dstlen ); > + if (flags & MB_USEGLYPHCHARS) add_glyphs( dst, dstlen ); > + return res; Mixed tabs/spaces. > - return mbstowcs_dbcs_decompose( &table->dbcs, src, srclen, dst, dstlen ); > + res = mbstowcs_dbcs_decompose( &table->dbcs, src, srclen, dst, dstlen ); > + if (flags & MB_USEGLYPHCHARS) add_glyphs( dst, dstlen ); > + return res; Again mixed tabs/spaces. -- Dmitry.

1 0

Re: Correct VarBstrFromDate for dates between DATE_MIN and 1601
by Bill Medland May 9, 2007

May 9, 2007

On Fri, 2007-04-05 at 11:22 -0700, Bill Medland wrote: > Bill Medland (billmedland(a)shaw.ca) > Correct VarBstrFromDate for dates between DATE_MIN and 1601 > Has this been rejected out of hand or is it still being considered? Bill

2 4

Re: [5/5] WineD3D: Manage private data in a wine linked list
by H. Verbeet May 9, 2007

May 9, 2007

On 09/05/07, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > Fixes an endless loop in the startup of Traffic Giant. > Shouldn't you free the private data when the resource is destroyed? (MSDN claims you should).

2 2

Re: [1/5] wined3d: Unload vertex weight data in case of EXT_VERTEX_WEIGHTING as well
by Stefan Dösinger May 9, 2007

May 9, 2007

Am Mittwoch 09 Mai 2007 19:07 schrieb H. Verbeet: > This set applies on top of Stefan's > > Changelog: > - Unload vertex weight data in case of EXT_VERTEX_WEIGHTING as well Actually I was thinking about removing the whole (disabled) vertex weighting code since I didn't find any card supporting this extension.

1 0

Re: [msi/tests] Properly cleanup after each test
by James Hawkins May 9, 2007

May 9, 2007

On 5/8/07, Paul Vriens <paul.vriens.wine(a)gmail.com> wrote: > Hi, > > All files in the destination directory (usually 'Program Files') are cleaned up, > but sometimes not the files/directories in the temp directory. We have to make > sure we have a clean start for every test. > > I've also changed a filename from 'path' to a clearer one (me thinks). > - CHAR path[MAX_PATH]; + CHAR msifile2[MAX_PATH]; CreateDirectoryA("msitest", NULL); CreateDirectoryA("msitest\\msitest", NULL); @@ -1451,9 +1460,9 @@ static void test_concurrentinstall(void) create_database(msifile, ci_tables, sizeof(ci_tables) / sizeof(msi_table)); - lstrcpyA(path, CURR_DIR); - lstrcatA(path, "\\msitest\\concurrent.msi"); - create_database(path, ci2_tables, sizeof(ci2_tables) / sizeof(msi_table)); + lstrcpyA(msifile2, CURR_DIR); + lstrcatA(msifile2, "\\msitest\\concurrent.msi"); + create_database(msifile2, ci2_tables, sizeof(ci2_tables) / sizeof(msi_table)); Please don't change the names of variables. I chose the generic name of path (which is used throught Wine), because that's exactly what the variable contains throughout the functions. Also, msifile2 is a poor choice of a variable name. Thanks, James Hawkins

2 1

Re: [PATCH] Uninstaller: Change hardcoded value for maximum subkey name length to a const.
by Dan Hipschman May 9, 2007

May 9, 2007

On Tue, May 08, 2007 at 11:19:16AM -0500, Tom Spear wrote: > static const DWORD MAXSUBKEYNAMELEN = 255; > WCHAR subKeyName[MAXSUBKEYNAMELEN + 1]; I'm not sure what version of C wine is strictly trying to conform to, but it's usually best to go with the lowest common denominator. This is not C89. The const keyword doesn't actually make the variable constant (I know it's not the greatest feature of C). The array size won't be evaluated until runtime, so technically this is a variable length array which weren't added until C99. You can make it C89 simply by using #define instead of declaring it as a variable.

2 1

Re: crypt32: remove hardcoded value AT_SIGNATURE from CertContext_SetKeyProvInfo
by Alexandre Julliard May 9, 2007

May 9, 2007

Mounir IDRASSI <mounir.idrassi(a)idrix.fr> writes: > Hi, > This patch removed a hardcoded value (AT_SIGNATURE) from > CertContext_SetKeyProvInfo. It also corrects a corresponding test. Like I said, the test currently passes on Windows, so you can't simply change it. Tests are supposed to reflect the behavior of Windows, not of Wine. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH 3/3] winex11: Use TINN algorithm to speed up colour lookups. (try 2)
by Dmitry Timoshkov May 9, 2007

May 9, 2007

"Vitaly Budovski" <vbudovski(a)gmail.com> wrote: >> Now that you got rid of sqrt calls usage of float numbers internally >> doesn't look justified (to me) anymore. >> > > Only because in this instance it is used with integer data. It doesn't > need to be limited to just integer values. Besides, what would be gained > by changing it to integers as you suggest? What would be gained is an additional speed. Since this code is supposed to be used to handle palette/color data there is no need to use floats at all. -- Dmitry.

5 8

Re: rsaenh: add support for mandatory parameters to RSAENH_CPGetProvParam
by Alexandre Julliard May 9, 2007

May 9, 2007

Mounir IDRASSI <mounir.idrassi(a)idrix.fr> writes: > This patch adds support for mandatory parameters in > RSAENH_CPGetProvParam. It also adds tests for them. > The mandatory parameters are: PP_UNIQUE_CONTAINER, PP_PROVTYPE, > PP_KEYSPEC, PP_KEYSET_TYPE, PP_KEYSTORAGE. Thanks for resending, but unfortunately this breaks make test: ../../../tools/runtest -q -P wine -M crypt32.dll -T ../../.. -p crypt32_test.exe.so cert.c && touch cert.ok cert.c:1215: Test failed: Expected AT_SIGNATURE, got 3 make: *** [cert.ok] Error 1 I know you sent a patch for that test, but since it currently succeeds on Windows the change can't be right. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH 05/27] [Kernel32]: ActCtx: now storing path info for app & config in manifest
by Alexandre Julliard May 9, 2007

May 9, 2007

Eric Pouech <eric.pouech(a)wanadoo.fr> writes: > HeapFree(GetProcessHeap(), 0, actctx->assemblies); > - actctx->magic = 0; > + HeapFree(GetProcessHeap(), 0, (void*)actctx->config.info); > + HeapFree(GetProcessHeap(), 0, (void*)actctx->app.info); Please don't cast off const, we are trying to get rid of these warnings. If the strings need to be freed then they cannot be const. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 03/27] [Kernel32]: ActCtx: added basic structures for storing assembly information
by Alexandre Julliard May 9, 2007

May 9, 2007

Eric Pouech <eric.pouech(a)wanadoo.fr> writes: > +static struct assembly* add_assembly(struct actctx* actctx, enum assembly_type at) > +{ > + struct assembly* assembly; > + > + if (!actctx->num_assemblies) > + actctx->assemblies = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(struct assembly)); > + else > + actctx->assemblies = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + actctx->assemblies, > + (actctx->num_assemblies + 1) * sizeof(struct assembly)); Growable arrays should be grown by a multiple of the current size. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: hnetcfg: [1:10]Define icftype.idl
by Alexandre Julliard May 9, 2007

May 9, 2007

Jeff Latimer <lats(a)yless4u.com.au> writes: > +typedef enum NET_FW_PROFILE_TYPE_ > +{ > + NET_FW_PROFILE_DOMAIN = 1, > + NET_FW_PROFILE_STANDARD = 2, > + NET_FW_PROFILE_CURRENT = 3, > + NET_FW_PROFILE_TYPE_MAX = 4 > +} NET_FW_PROFILE_TYPE; > + > +typedef enum NET_FW_IP_VERSION_ > +{ > + NET_FW_IP_VERSION_V4 = 1, > + NET_FW_IP_VERSION_V6 = 2, > + NET_FW_IP_VERSION_ANY = 3, > + NET_FW_IP_VERSION_MAX = 4 > +} NET_FW_IP_VERSION; There's no reason to initialize the enum values explicitly, particularly since all your values are wrong... -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: rsaenh: fix bug in RSAENH_CPGetProvParam (corrected again)
by Alexandre Julliard May 9, 2007

May 9, 2007

Mounir IDRASSI <mounir.idrassi(a)idrix.fr> writes: > From: Mounir IDRASSI <mounir.idrassi(a)idrix.fr> > Date: Wed, 9 May 2007 03:33:45 +0200 > Subject: rsaenh: fix bug in RSAENH_CPGetProvParam Could you please provide a more meaningful subject line to describe your patch? "fix bug" applies to just about every commit... -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: shell32: handle invalid cbSize in Shell_NotifyIcon[AW] (fixed, with testcase)
by Alexandre Julliard May 9, 2007

May 9, 2007

Mikołaj Zalewski <mikolaj(a)zalewski.pl> writes: > As I've understood from wine-devel to catch exceptions in tests I > need to write my own handler. My handler is more compilcated than in > ntdll/tests/exception.c as I can't assume the next instruction after > the invalid one will be a 'ret' but is simplier than wine > __TRY/__EXCEPT as I don't try to filter exception but catch all of > them (I hope this is not a problem with stack guard pages or something > like that). I'm also not using siglongjump but longjmp to be able to > compile it with msvcrt. It won't work when not using msvcrt, and it's really too ugly. That sort of thing might be acceptable in an exception test in ntdll, but not in shell32 tests. Please find another way to write the test that doesn't rely on deliberately triggering a crash. -- Alexandre Julliard julliard(a)winehq.org

1 0

RE: Resend of Adds NET START to net.exe
by Rolf Kalbermatter May 9, 2007

May 9, 2007

Tim Schwartz wrote: + if(!strcasecmp(operation, "start")) + { + printf("The %s service is starting.\n", service_display_name); + result = StartService(serviceHandle,0,NULL); + CloseServiceHandle(serviceHandle); + if(!result) printf("The %s service failed to start.\n", service_display_name); + else printf("The %s service was started successfully.\n", service_display_name); + + HeapFree(GetProcessHeap(), 0, service_display_name); + CloseServiceHandle(SCManager); + return result; + } + + CloseServiceHandle(serviceHandle); + CloseServiceHandle(SCManager); + HeapFree(GetProcessHeap(), 0, service_display_name); + return 0; +} I think with a little rearrangment of the code and returning result at the end you could avoid duplicating those CloseServiceHandle() and HeapFree() calls. int main(int argc, char *argv[]) { - int ret = 0; if (argc < 2) { printf("The syntax of this command is:\n\n"); - printf("NET [ HELP ]\n"); + printf("NET [ HELP | START ]\n"); return 1; } @@ -35,7 +88,24 @@ int main(int argc, char *argv[]) printf("The syntax of this command is:\n\n"); printf("NET HELP command\n -or-\nNET command /HELP\n\n"); printf(" Commands available are:\n"); - printf(" NET HELP\n"); + printf(" NET HELP NET START\n"); } - return ret; + + if(!strcasecmp(argv[1], "start")) + { + if(argc < 3) + { + printf("Specify service name to start.\n"); + return 1; + } + + if(!net_service(argv[1], argv[2])) + { + return 1; + } + return 0; + } + + + return 0; } Me thinks it would be cleaner to only string test for the command in the main() and pass in a constant you define somewhere to net_service() that specifies the operation to perform. That will allow you to use a switch statement in net_service() and make the code a lot easier to read. Also adding new commands won't make the whole such a big mess as it would now. Also consider to use TRUE and FALSE as return value. This being a Windows tool and not a Unix one it seems clearer to me. Rolf Kalbermatter

1 0

Drop-in replacement of windows dll's?
by Tom Spear May 9, 2007

May 9, 2007

I just remembered reading somewhere (wiki, perhaps?) that the ultimate goal of wine (aside from documenting the windows api, etc) was to provide open-source drop-in replacements for windows' core dll's. If that is the case, do we have a framework setup for building the wine dlls as windows .dll files, so that we could test the actual status of this? I think it would be interesting to see how much of windows will run with wine's dlls in their current state. Obviously it would be bad to test this with something like user32.dll, etc, but something like riched32 and maybe some of the common controls dlls should be ok to test with? -- Thanks Tom Check out this new 3D Instant Messenger called IMVU. It's the best I have seen yet! http://imvu.com/catalog/web_invitation.php?userId=1547373&from=power-email

4 4

Re: Resend of Adds NET START to net.exe
by Dmitry Timoshkov May 9, 2007

May 9, 2007

"Tim Schwartz" <tim(a)sanityinternet.com> wrote: > + GetServiceDisplayName(SCManager, service_name, NULL, &buffer_size); You need to check GetServiceDisplayName return value here and later. > + /* until GetServiceDisplayName() is implemented, buffer_size will always be zero so the service short name will > be used instead */ GetServiceDisplayName *is* implemented. > + if(!buffer_size) > + { The check above is wrong. -- Dmitry.

1 1

rsaenh: fix bug in RSAENH_CPGetProvParam (corrected)
by Juan Lang May 9, 2007

May 9, 2007

Hi Mounir, + case PP_KEYSTORAGE: + dwTemp = CRYPT_SEC_DESCR; This looks fine.. + result = CryptGetProvParam(prov, PP_KEYSTORAGE, (LPBYTE)&dwParam, &dataLen, 0); + ok(result && dataLen == sizeof(dwParam) && (dwParam == CRYPT_SEC_DESCR), + "Expected CRYPT_SEC_DESCR to be set, got 0x%08X\n",dwParam); but this test will now fail on Win9x, yes? A simple fix would be to check (dwParam & CRYPT_SEC_DESCR) rather than (dwParam == CRYPT_SEC_DESCR). --Juan ____________________________________________________________________________________ Don't pick lemons. See all the new 2007 cars at Yahoo! Autos. http://autos.yahoo.com/new_cars.html

2 3