January 2013 - Wine-devel - list.winehq.org

Re: user32/tests: Test standard scrollbar's initialization (try 2).
by Dmitry Timoshkov 22 Jan '13

22 Jan '13

Sergey Guralnik <serhio(a)etersoft.ru> wrote: > Subject: user32/tests: Test standard scrollbar's initialization (try 2). If it matters the test looks good to me. -- Dmitry.

1 0

Re: [PATCH] windowscodecs: Add support for DIB format.
by Dmitry Timoshkov 22 Jan '13

22 Jan '13

Christian Costa <titan.costa(a)gmail.com> wrote: > cpp_quote("DEFINE_GUID(CLSID_WICBmpDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x78);") > +cpp_quote("DEFINE_GUID(CLSID_WICDibDecoder, 0x6b462062,0x7cbf,0x400d,0x9f,0xdb,0x81,0x3d,0xd1,0x0f,0x27,0x79);") > cpp_quote("DEFINE_GUID(CLSID_WICPngDecoder, 0x389ea17b,0x5078,0x4cde,0xb6,0xef,0x25,0xc1,0x51,0x75,0xc7,0x51);") > cpp_quote("DEFINE_GUID(CLSID_WICIcoDecoder, 0xc61bfcdf,0x2e0f,0x4aad,0xa8,0xd7,0xe0,0x6b,0xaf,0xeb,0xcd,0xfe);") > cpp_quote("DEFINE_GUID(CLSID_WICJpegDecoder, 0x9456a480,0xe88b,0x43ea,0x9e,0x73,0x0b,0x2d,0x9b,0x71,0xb1,0xca);") > @@ -1002,6 +1003,7 @@ cpp_quote("DEFINE_GUID(CLSID_WICWmpEncoder, 0xac4ce3cb,0xe1c1,0x44cd,0x82,0x15,0 > cpp_quote("DEFINE_GUID(CLSID_WICDefaultFormatConverter, 0x1a3f11dc,0xb514,0x4b17,0x8c,0x5f,0x21,0x54,0x51,0x38,0x52,0xf1);") > > cpp_quote("DEFINE_GUID(GUID_ContainerFormatBmp, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe3);") > +cpp_quote("DEFINE_GUID(GUID_ContainerFormatDib, 0x0af1d87e,0xfcfe,0x4188,0xbd,0xeb,0xa7,0x90,0x64,0x71,0xcb,0xe4);") Is this mocked up names? I couldn't find WIC DIB decoder definitions in the PSDK headers. -- Dmitry.

3 10

Re: macdrv: implement getting and setting the screen saver state, version 4
by Ken Thomases 22 Jan '13

22 Jan '13

Hi, On Jan 21, 2013, at 6:21 PM, C.W. Betts wrote: > This version falls through in SPI_SETSCREENSAVEACTIVE. Sadly, you used C++ style for the fall-through comment. :-/ Also, you have some trailing whitespace and the patch no longer applies cleanly after today's commits of mine. It should be easy to fix that though. There aren't any serious conflicts. -Ken

1 0

Adding DECLSPEC_ALIGN(4) to structs for atomic DWORD access
by Joerg-Cyril.Hoehle＠t-systems.com 22 Jan '13

22 Jan '13

Hi, there are some places in Wine where I believe atomic access to 32bit entities is expected by the programmer, yet I see little or no measures to ensure this. DWORD-Alignment is essential when several threads can access the same DWORD variable outside of a mutex (or within the implementation of the mutex). DECLSPEC_ALIGN(4) is the declaration that could be used to enforce it within a struct, assuming the struct itself is somehow aligned. E.g. shouldn't CRITICAL_SECTION objects be aligned when used within other (non-public) structs? (Presumably public MS structs cannot be augmented with alignment declarations) DECLSPEC_ALIGN is hardly used in Wine http://source.winehq.org/ident?i=RTL_CRITICAL_SECTION Other examples where I'm considering adding DWORD-alignment to read DWORD-sized objects atomically: - MCI position values (for MCI_STATUS_POSITION) - MIDI end_thread control variable - mmdevapi GetCurrentPadding (as part of lock-less patch) Would such patches be welcome? Are they superfluous, assuming that DWORD-alignment is granted and using DECLSPEC_ALIGN() only for larger alignments, e.g. 8 or 16 (that seems to be the current use)? One benefit I see is to highlight those variables shared among several threads. Regards, Jörg Höhle

3 2

Re: x11drv: Return true when handling SPI_SETSCREENSAVEACTIVE
by Alexandre Julliard 22 Jan '13

22 Jan '13

"C.W. Betts" <computers57(a)hotmail.com> writes: > I noticed in my research that the X11 driver doesn't return true in > SystemParametersInfo when handling SPI_SETSCREENSAVEACTIVE. This patch > fixes this. That's on purpose, it also needs to update the registry entry. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: macdrv: implement getting and setting the screen saver state, version 3
by Ken Thomases 21 Jan '13

21 Jan '13

On Jan 21, 2013, at 11:50 AM, C.W. Betts wrote: > This version gets rid of redundant comments. According to Alexandre's comment on your X11 driver patch, you should be returning FALSE from the "SET" case so that user32 will update the registry. Otherwise, looking good. -Ken

1 0

Re: [PATCH] winebuild: Use Clang to assemble if found.
by Alexandre Julliard 21 Jan '13

21 Jan '13

Charles Davis <cdavis5x(a)gmail.com> writes: > @@ -319,26 +341,27 @@ char *find_tool( const char *name, const char * const *names ) > names = alt_names; > } > > - while (*names) > + /* If we have a target triple, we really need to try the list of names > + * twice: once with the triple, and once without. > + */ This doesn't make sense, if we have a target it means we are cross-compiling, and we need to use the correct tool. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: macdrv: implement getting and setting the screen saver state, version 2
by Ken Thomases 21 Jan '13

21 Jan '13

Hi, On Jan 20, 2013, at 2:00 PM, C.W. Betts wrote: > This version implements changes and advice from Ken Thomases, including using kIOPMAssertionTypePreventUserIdleDisplaySleep on Lion and later. Also, some comments were added. > + //Get pre-Lion no display sleep counts C++-style comments ("//") are not allowed in Wine C code. They aren't portable. > + CFNumberRef count = CFDictionaryGetValue(assertionStates, kIOPMAssertionTypeNoDisplaySleep); > + CFNumberRef count2 = NULL; > + long longCount = 0; > + long longCount2 = 0; > + CFNumberGetValue(count, kCFNumberLongType, &longCount); > + //If available, get Lion and later no display sleep counts > + count2 = CFDictionaryGetValue(assertionStates, kIOPMAssertionTypePreventUserIdleDisplaySleep); > + if (count2) > + CFNumberGetValue(count2, kCFNumberLongType, &longCount2); The issue discussed for the previous patch is still present. For example, the following code would be internally consistent and also safe: CFNumberRef count = CFDictionaryGetValue(assertionStates, kIOPMAssertionTypeNoDisplaySleep); CFNumberRef count2 = CFDictionaryGetValue(assertionStates, kIOPMAssertionTypePreventUserIdleDisplaySleep); long longCount = 0; long longCount2 = 0; if (count) CFNumberGetValue(count, kCFNumberLongType, &longCount); if (count2) CFNumberGetValue(count2, kCFNumberLongType, &longCount2); (Note that this reorganization also eliminates a dead store. You were initializing count2 and then unconditionally assigning it, making the initialization redundant. Such unnecessary initializations also defeat potentially valuable compiler warnings.) > + if (longCount || longCount2 ) There's an extraneous space before the closing parenthesis. > + //Release power assertion > + IOReturn success = IOPMAssertionRelease(powerAssertion); The comment is useless. It tells us nothing that the code doesn't. Please don't do that. In fact, most of the comments you added are like this. > + //Are we running Lion or later? > + if (kCFCoreFoundationVersionNumber >= kCFCoreFoundationVersionNumber10_7) > + //If so, use Lion's name > + assertName = kIOPMAssertionTypePreventUserIdleDisplaySleep; > + else > + //If not, use old name > + assertName = kIOPMAssertionTypeNoDisplaySleep; I'm not sure it's correct that these are two different names for roughly the same thing. I think the two assertion types do slightly different things. "NoDisplaySleep" seeks to prevent display sleep for any (non-forced) reason. "PreventUserIdleDisplaySleep" only seeks to prevent display sleep that is due to user idleness. As I suggested previously, I do want us to use PreventUserIdleDisplaySleep when it's available, so that's good. (Thank you for making that change.) What I'm saying now is mostly just that the comments are misleading. I suggest just removing the comments within the "if". The code is understandable without them. Also, this bit of code uses tabs to indent when none of the rest does. Please be consistent with the surrounding code. Thanks, Ken

2 2

Re: configure.ac: check for a good enough version of libv4l1.h (try 2)
by Alexandre Julliard 21 Jan '13

21 Jan '13

Damjan Jovanovic <damjan.jov(a)gmail.com> writes: > @@ -676,6 +676,22 @@ AC_CHECK_HEADERS([linux/videodev.h linux/videodev2.h libv4l1.h],,, > #include <asm/types.h> > #endif]) > > +dnl **** Check for libv4l1.h **** > +if test "x$with_v4l" != "xno" > +then > + ac_save_CPPFLAGS="$CPPFLAGS" > + if test "$PKG_CONFIG" != "false" > + then > + ac_libv4l1_libs="`$PKG_CONFIG --libs libv4l1 2>/dev/null`" > + ac_libv4l1_cflags="`$PKG_CONFIG --cflags libv4l1 2>/dev/null`" > + CPPFLAGS="$CPPFLAGS $ac_libv4l1_cflags" > + AC_CHECK_TYPES([struct video_picture],,,[#include <libv4l1.h>]) > + fi > + CPPFLAGS="$ac_save_CPPFLAGS" > + WINE_NOTICE_WITH(libv4l1,[test "x$ac_cv_type_struct_video_picture" = "x"], > + [libv4l1 ${notice_platform}development files not found or too old.]) > +fi It doesn't make sense to use pkg-config for the test since we don't use it when including the header. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [1/2] netstat: Implement the TCP stats output
by Frédéric Delanoy 21 Jan '13

21 Jan '13

On Sun, Jan 20, 2013 at 8:09 PM, André Hentschel <nerv(a)dawncrow.de> wrote: > --- > + IDS_TCP_SEGM_RECV, "Segments Recieved" Typo here: should be "Received" (same issue on the 2nd patch) Frédéric

1 0