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 9, 2007

May 9, 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