February 2009 - Wine-Devel - list.winehq.org

Re: ntdll: add a warning about running wine as root (resend)

by Austin English

On Tue, Feb 10, 2009 at 8:38 AM, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > Alexandre Julliard wrote: >> Vitaliy Margolen <wine-devel(a)kievinfo.com> writes: >> >>> Alexandre Julliard wrote: >>>> Again, please demonstrate the exact sequence that leads to an actual >>>> problem, not just vague hearsay of people reporting problems that may or >>>> may not have anything to do with this. >>>> >>> Will this work? BTW default Open SuSE configuration. >>> >>> vitaliy(a)dragon:~ $export WINEPREFIX=~/.wine-root >>> vitaliy(a)dragon:~ $sudo wine winecfg >>> root's password: >>> wine: created the configuration directory '/root/.wine' >> >> As you can see it's creating a new prefix under /root, so it's not >> messing up the user's prefix. That's perfectly fine. >> > That part worked you are correct. > > However X11 part didn't (missing $DISPLAY). And even if $DISPLAY would be > defined in this case all programs will be installed into root's env not > user's. No menu entries, no desktop links no visible installed programs - > big problem for noob users "where did my stuff go"? > > Vitaliy. > > > That's not the (main) problem. The main problem is people installing stuff as root when they have no reason to do so. Alexandre, how would you feel about a one time warning, e.g., by setting a registry key, a la wineboot when run on a new/updated prefix. -- -Austin

Feb. 10, 2009

Re: ntdll: add a warning about running wine as root (resend)

by Vitaliy Margolen

Alexandre Julliard wrote: > Vitaliy Margolen <wine-devel(a)kievinfo.com> writes: > >> Alexandre Julliard wrote: >>> Again, please demonstrate the exact sequence that leads to an actual >>> problem, not just vague hearsay of people reporting problems that may or >>> may not have anything to do with this. >>> >> Will this work? BTW default Open SuSE configuration. >> >> vitaliy(a)dragon:~ $export WINEPREFIX=~/.wine-root >> vitaliy(a)dragon:~ $sudo wine winecfg >> root's password: >> wine: created the configuration directory '/root/.wine' > > As you can see it's creating a new prefix under /root, so it's not > messing up the user's prefix. That's perfectly fine. > That part worked you are correct. However X11 part didn't (missing $DISPLAY). And even if $DISPLAY would be defined in this case all programs will be installed into root's env not user's. No menu entries, no desktop links no visible installed programs - big problem for noob users "where did my stuff go"? Vitaliy.

Feb. 10, 2009

Re: [PATCH] gdi32: handle failure in __wine_make_gdi_object_system (Coverity)

by Alexandre Julliard

Marcus Meissner <marcus(a)jet.franken.de> writes: > @@ -445,6 +445,11 @@ static const struct DefaultFontInfo default_fonts[] = > void CDECL __wine_make_gdi_object_system( HGDIOBJ handle, BOOL set) > { > GDIOBJHDR *ptr = GDI_GetObjPtr( handle, 0 ); > + > + if (!ptr) { > + ERR("System object for handle %p not found.\n", handle); > + return; > + } That shouldn't happen, it's an internal function and the caller is supposed to pass a valid handle. -- Alexandre Julliard julliard(a)winehq.org

Feb. 10, 2009

Re: mshtml/tests: Better check for installed Gecko.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -4199,9 +4199,8 @@ static void run_domtest(const char *str, domtest_t test) > } > > hres = IHTMLDocument2_get_body(doc, &body); > - ok(hres == S_OK, "get_body failed: %08x\n", hres); > - > - if(body) { > + if(hres != E_UNEXPECTED) { > + ok(hres == S_OK, "get_body failed: %08x\n", hres); > IHTMLElement_Release(body); We've been through this before, there needs to be at least one failure if mshtml is non-functional because Gecko is missing. I'd suggest to remove the Gecko install workaround and have the test force the install in all cases, it's not useful to run the tests without Gecko anyway. -- Alexandre Julliard julliard(a)winehq.org

Feb. 10, 2009

Re: itss: Replace malloc() with HeapAlloc()

by Francois Gouget

On Tue, 10 Feb 2009, Nikolay Sivov wrote: [...] > Why didn't you replace corresponding free() calls? Also, for everyone's information, there's more calls to malloc() and free(). I only grepped in the dlls/ directory where they are the most suspicious. I removed some obvious false positives but there may be more. Here's what I got: dlls/itss/lzx.c:181: pState = malloc(sizeof(struct LZXstate)); dlls/itss/lzx.c:182: if (!(pState->window = malloc(wndsize))) dlls/rsaenh/mpi.c:157: a->dp = malloc (sizeof (mp_digit) * MP_PREC); dlls/rsaenh/mpi.c:185: a->dp = malloc (sizeof (mp_digit) * size); dlls/rsaenh/mpi.c:3381: tmp = malloc(bsize); dlls/twain_32/dsm_ctrl.c:89: devices = malloc(sizeof(devices[0])); dlls/winedos/dosconf.c:267: DOSCONF_config.country = malloc(strlen(*confline) + 1); dlls/winedos/dosconf.c:307: DOSCONF_config.shell = malloc(strlen(*confline) + 1); dlls/winedos/dosconf.c:370: DOSCONF_menu_default = malloc(strlen(*confline) + 1); dlls/winedos/dosconf.c:387: temp = malloc(strlen(*confline) + 1); dlls/winedos/dosvm.c:260: event = malloc(sizeof(DOSEVENT)); dlls/winedos/ppdev.c:184: PPDeviceList[nports].devicename = malloc(sizeof(buffer)+1); dlls/winemp3.acm/interface.c:74: nbuf = malloc( sizeof(struct buf) ); dlls/winemp3.acm/interface.c:79: nbuf->pnt = malloc(size); dlls/winenas.drv/audio.c:1383: void *newbuf = malloc(wwo->BufferUsed + len); dlls/winenas.drv/audio.c:1418: ptr = malloc(len); dlls/winenas.drv/audio.c:1420: newdata = malloc(wwo->BufferUsed - len); dlls/winhttp/net.c:634: * SSL 0.9.7 and above malloc the buffer if it is null. dlls/winhttp/net.c:653: if (malloc) free( buffer ); dlls/ddraw/tests/ddrawmodes.c:95: modes = malloc((modes_size = 2) * sizeof(DDSURFACEDESC)); dlls/dsound/tests/ds3d.c:51: b=buf=malloc(*size); dlls/lz32/tests/lzexpand_main.c:884: buf = malloc(uncompressed_data_size * 2); dlls/oleaut32/tests/vartype.c:35:# include <malloc.h> dlls/rpcrt4/tests/server.c:1202: dc = malloc(FIELD_OFFSET(doub_carr_t, a[2])); dlls/rpcrt4/tests/server.c:1204: dc->a[0] = malloc(FIELD_OFFSET(doub_carr_1_t, a[3])); dlls/rpcrt4/tests/server.c:1209: dc->a[1] = malloc(FIELD_OFFSET(doub_carr_1_t, a[2])); dlls/rpcrt4/tests/server.c:56: return malloc(n); dlls/shell32/tests/shlexec.c:336: cmd=malloc(strlen(argv0)+10+strlen(child_file)+2+strlen(cmdtail)+1); dlls/dbghelp/msc.c:2480: free(files_image); dlls/itss/lzx.c:184: free(pState); dlls/itss/lzx.c:220: free(pState->window); dlls/itss/lzx.c:221: free(pState); dlls/ntdll/server.c:752: free( tmp_dir ); dlls/rsaenh/mpi.c:217: free(a->dp); dlls/rsaenh/mpi.c:3446: free(tmp); dlls/winedos/dosconf.c:357: free(DOSCONF_menu_default); dlls/winedos/dosconf.c:390: free(temp); dlls/winedos/dosvm.c:185: free(event); dlls/winedos/dosvm.c:625: free(event); dlls/winedos/int33.c:221: free(data); dlls/winejack.drv/audio.c:2006: free(ports); /* free the returned array of ports */ dlls/winejack.drv/audio.c:735: free(ports); /* free the returned array of ports */ dlls/winemp3.acm/interface.c:115: free(buf->pnt); dlls/winemp3.acm/interface.c:116: free(buf); dlls/winemp3.acm/interface.c:60: free(b->pnt); dlls/winemp3.acm/interface.c:62: free(b); dlls/winemp3.acm/interface.c:81: free(nbuf); dlls/winenas.drv/audio.c:1387: free(oldbuf); dlls/winenas.drv/audio.c:1432: free(ptr); dlls/wininet/netconnection.c:753: free(buffer); dlls/ddraw/tests/ddrawmodes.c:104: free(modes); dlls/dsound/tests/ds3d8.c:521: free(state.wave); dlls/dsound/tests/ds3d.c:677: free(state.wave); dlls/lz32/tests/lzexpand_main.c:889: free(buf); dlls/ntdll/tests/rtlstr.c:1367: free(wstr); dlls/ntdll/tests/rtlstr.c:1407: free(wstr); dlls/ntdll/tests/rtlstr.c:296: free(teststring2); dlls/rpcrt4/tests/server.c:1214: free(dc->a[0]); dlls/rpcrt4/tests/server.c:1215: free(dc->a[1]); dlls/rpcrt4/tests/server.c:1216: free(dc); dlls/rpcrt4/tests/server.c:62: free(p); dlls/shell32/tests/shlexec.c:340: free(cmd); Notes: * It's probably not as bad to use malloc() in tests but it should still be avoided. * Using #include <malloc.h> when not using malloc is wrong. * Some calls to malloc() and free() may be required by the Unix APIs that are called. This should be checked on a case by case basis. * If the above don't have too many false positives I could integrate that check into my scripts. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ A polar bear is a cartesian bear after a coordinate transform.

Feb. 10, 2009

Re: ntdll: add a warning about running wine as root (resend)

by Francois Gouget

On Tue, 10 Feb 2009, Ben Klein wrote: [...] > > Application tried to create a window, but no driver could be loaded. > > Make sure that your X server is running and that $DISPLAY is set correctly. > Interesting. xauth problem. Again, I'd be surprised if this happened > with sudo. Not so surprised if it happened with "su -". X access will very often be broken after a sudo or an su (e.g. Debian). In fact I believe it will only work on distributions which install special PAM packages. For all others you should use gksu, kdesu or sux. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Cahn's Axiom: When all else fails, read the instructions.

Feb. 10, 2009

Re: spoolss: Implement EnumPorts [try 2]

by Alexandre Julliard

Detlef Riekenberg <wine.dev(a)web.de> writes: > Synced to current git. It doesn't work here: ../../../tools/runtest -q -P wine -M winspool.drv -T ../../.. -p winspool.drv_test.exe.so info.c && touch info.ok info.c:505: Test failed: got 1 with 127 (expected '0' with ERROR_INVALID_PARAMETER) make: *** [info.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

Feb. 10, 2009

Re: [PATCH] winegcc: Free the temporary string arrays (Coverity)

by Alexandre Julliard

Michael Stefaniuc <mstefani(a)redhat.com> writes: > Alexandre Julliard wrote: >> Marcus Meissner <marcus(a)jet.franken.de> writes: >> >>> This fixes CID 883, 882, 881, 880, 879, 878, 877, >>> which are all basically missing strarray_free(arr)s. >> >> winegcc is a short-lived program, there's no reason to care about memory >> leaks. > Right but the problem is that people will keep running into that. This > is not the first attempt to fix those; maybe the 4th or even 5th one. > So while it isn't necessary code wise it will cut on developer time > having to deal again and again with the same "false positives". > > If the code doesn't get too ugly I would vote to get the patch in. There are many more leaks in there, the whole program (and the whole tools/ directory really) is written with the assumption that you don't have to free memory. That makes things a lot easier so it's worth it. -- Alexandre Julliard julliard(a)winehq.org

Feb. 10, 2009

Re: [PATCH] winegcc: Free the temporary string arrays (Coverity)

by Michael Stefaniuc

Alexandre Julliard wrote: > Marcus Meissner <marcus(a)jet.franken.de> writes: > >> This fixes CID 883, 882, 881, 880, 879, 878, 877, >> which are all basically missing strarray_free(arr)s. > > winegcc is a short-lived program, there's no reason to care about memory > leaks. Right but the problem is that people will keep running into that. This is not the first attempt to fix those; maybe the 4th or even 5th one. So while it isn't necessary code wise it will cut on developer time having to deal again and again with the same "false positives". If the code doesn't get too ugly I would vote to get the patch in. bye michael

Feb. 10, 2009

Re: [PATCH] winegcc: Free the temporary string arrays (Coverity)

by Alexandre Julliard

Marcus Meissner <marcus(a)jet.franken.de> writes: > Hi, > > This fixes CID 883, 882, 881, 880, 879, 878, 877, > which are all basically missing strarray_free(arr)s. winegcc is a short-lived program, there's no reason to care about memory leaks. -- Alexandre Julliard julliard(a)winehq.org

Feb. 10, 2009