Re: New Wine stuff in FreeWear.org
Hi again, As the design seems to be liked, we just made the garments available for sale. As always, you can find them at http://www.freewear.org/?org=Wine Cheers! Ismael
Re: [PATCH 2/7] dpwsockx: Implementation of SPInit
On Fri, Sep 4, 2009 at 12:12 PM, Henri Verbeethverb...@gmail.com wrote:
2009/9/4 Ismael Barros² razielm...@gmail.com:
I've been looking into iphlpapi/ip.h (just learned bit fields
exist...); would this implementation be fine?
#include pshpack1.h
typedef struct tagDPSP_MSG_HEADER
{
#ifdef WORDS_BIGENDIAN
DWORD size:20;
DWORD token:12;
#else
DWORD token:12;
DWORD size:20;
#endif
SOCKADDR_IN SockAddr;
} DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
Probably not. I'm not sure iphlpapi/ip.h does things right in the
first place, it seems to confuse bitfield ordering with machine byte
ordering. As a general rule, I think it's best to avoid bitfields for
things like writing data to a file or sending data over the network,
it's just a pain. Aside from that, using the bitfields in this way
messes with the bit ordering, but doesn't change the byte order. For
the bitfields in iphlpapi/ip.h that's not an issue because they fit in
a single byte. Just store the data in a DWORD with the appropriate
masks and shifts, and byte swap that DWORD when reading/writing it.
Here is the next try, how does it look?
It seems to behave correctly in some quick tests (byte-swapping
parameters to emulate big-endian).
Thanks a lot for the feedback, really appreciated.
-
#ifdef WORDS_BIGENDIAN
static inline u_short __dpws_ushort_swap(u_short s)
{
return (s 8) | (s 8);
}
static inline u_long __dpws_ulong_swap(u_long l)
{
return ((u_long)__dpws_ushort_swap((u_short)l) 16) |
__dpws_ushort_swap((u_short)(l 16));
}
#define dpws_letohl(l) __dpws_ulong_swap(l)
#define dpws_letohs(s) __dpws_ushort_swap(s)
#define dpws_htolel(l) __dpws_ulong_swap(l)
#define dpws_htoles(s) __dpws_ushort_swap(s)
#else /* WORDS_BIGENDIAN */
#define dpws_letohl(l) ((u_long)(l))
#define dpws_letohs(s) ((u_short)(s))
#define dpws_htolel(l) ((u_long)(l))
#define dpws_htoles(s) ((u_short)(s))
#endif /* WORDS_BIGENDIAN */
#include pshpack1.h
typedef struct tagDPSP_MSG_HEADER
{
DWORD mixed;
SOCKADDR_IN SockAddr;
} DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
typedef const DPSP_MSG_HEADER* LPCDPSP_MSG_HEADER;
#include poppack.h
#define DPSP_MSG_TOKEN_REMOTE0xFAB0
#define DPSP_MSG_TOKEN_FORWARDED 0xCAB0
#define DPSP_MSG_TOKEN_SERVER0xBAB0
#define DPSP_MSG_MAKE_MIXED(s,t) dpws_htolel((s) | (t))
#define DPSP_MSG_SIZE(m) dpws_letohl((m) 0x000F)
#define DPSP_MSG_TOKEN(m)((m) 0xFFF0)
Re: [PATCH 2/7] dpwsockx: Implementation of SPInit
On Tue, Sep 1, 2009 at 12:43 PM, Henri Verbeethverb...@gmail.com wrote:
2009/9/1 Ismael Barros² razielm...@gmail.com:
Is there any standard way to deal with this cases? Any example in the
codebase?
For packing, look for #include pshpack1.h and #include
poppack.h in some of the headers. For byte ordering look at how
htonl etc. are defined in include/winsock.h, but you'll want the
opposite since the data uses x86 byte ordering.
I've been looking into iphlpapi/ip.h (just learned bit fields
exist...); would this implementation be fine?
#include pshpack1.h
typedef struct tagDPSP_MSG_HEADER
{
#ifdef WORDS_BIGENDIAN
DWORD size:20;
DWORD token:12;
#else
DWORD token:12;
DWORD size:20;
#endif
SOCKADDR_IN SockAddr;
} DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
typedef const DPSP_MSG_HEADER* LPCDPSP_MSG_HEADER;
/* DPSP_MSG_HEADER-token */
#define DPSP_MSG_TOKEN_REMOTE0xFAB
#define DPSP_MSG_TOKEN_FORWARDED 0xCAB
#define DPSP_MSG_TOKEN_SERVER0xBAB
#include poppack.h
Re: Wine t-shirts?
Hi there, The T-shirt is finally on sale. We waited to have the Valgrind T-shirt ready to release both at the same time, as I guess many Wine lovers may also be Valgrind lovers. Direct link: http://www.freewear.org/?page=show_itemid=FW0030 If we have resources we'll try to upgrade our Wine catalog soon with other style of designs, stay tunned. Cheers, Ismael On Tue, Aug 4, 2009 at 11:45 PM, Ismael Barros²razielm...@gmail.com wrote: Hi there, Sorry for the huge delay, our artist has been quite busy. We finally got the last design iteration: http://www.freewear.org/images/release_candidates/Wine_final.png The only difference between versions A and B is in the eyes. Let us know which one you like better, and any kind of comment or suggestion is welcomed. After the design is approved, in one week we make have the T-shirt available in the store. Cheers, Ismael
Re: [PATCH 2/7] dpwsockx: Implementation of SPInit
On Tue, Sep 1, 2009 at 11:00 AM, Henri Verbeethverb...@gmail.com wrote:
2009/9/1 Ismael Barros razielm...@gmail.com:
+typedef struct tagDPSP_MSG_HEADER
+{
+ DWORD size; /* size 0x000F, token 0xFFF0 */
+ SOCKADDR_IN SockAddr;
+} DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
+typedef const DPSP_MSG_HEADER* LPCDPSP_MSG_HEADER;
+
It seems you're sending this over the network, so you should make sure
it's correctly packed, and take the byte order into account.
I tested it with a wireshark dissector and against the original
implementation, and it seems to work nicely.
Re: [PATCH 1/3] dpwsockx: Stub implementation
On Sat, Aug 29, 2009 at 12:25 PM, Stefan Dösingerstefandoesin...@gmx.at wrote:
Yay, Dplay work! A few suggestions though:
From patch 1:
+static HRESULT WINAPI DPWSCB_GetCaps( LPDPSP_GETCAPSDATA data )
+{
+ TRACE( (%d,%p,0x%08x,%p)\n,
+ data-idPlayer, data-lpCaps, data-dwFlags, data-lpISP );
+ return DP_OK;
+}
Is there any reason this one writes a TRACE instread of a FIXME? I noticed
that patch 3 implements it, perhaps you missed this when separating the
patches?
Hum, true, I missed it, but anyway it gets implemented two patches later.
+static HRESULT WINAPI DPWSCB_Open( LPDPSP_OPENDATA data )
+{
+ FIXME( (%u,%p,%p,%u,0x%08x,0x%08x) stub\n,
+ data-bCreate, data-lpSPMessageHeader, data-lpISP,
+ data-bReturnStatus, data-dwOpenFlags, data-dwSessionFlags );
+ return DP_OK;
+}
Why does it return DP_OK while most other stubs return an error?
I implemented it a year ago and I don't remember clearly that detail,
but it most probably was just to avoid dpwsoxks initialization failing
so I could keep fixing invalid parameter detection inside dplayx.
Anyway same issue than above, the function is implemented but it will
come in the next series of patches, when I finish reviewing them.
Patch 2:
+ /* Initialize internal data */
+ ZeroMemory( dpwsData, sizeof(DPWS_DATA) );
I think memset(dpwsData, 0, sizeof(DPWS_DATA)) is preferred over ZeroMemory.
I wasn't informed, but the codebase seems to confirm it:
[raz...@bebop dlls]$ grep -Pr memset\([^,]+,[ ]*0 * | wc -l
5565
[raz...@bebop dlls]$ grep -r ZeroMemory * | wc -l
660
That should probably go into the doc
(http://www.winehq.org/site/developer-cheatsheet), like the advice of
using HeapAlloc instead of malloc.
Patch 3:
Where do the values like dwMaxLocalPlayers = 65536 come from? Since most
functions are still stubs its hard to see where it comes from? Does native
dpwsockx have the same limits? (If it does, that's a solid reason for using
the same limits)
They come from the tests results, I copied them from the original
implementation. Some of them have a documented behaviour (like
substracting the size of the headers from the max buffer size), but
others seem to be arbitrary. They may need some tweaking depending on
the Windows version, but that will be shown over time by further
testing.
Thanks for the feedback :)
Re: Wine t-shirts?
Hi there, Sorry for the huge delay, our artist has been quite busy. We finally got the last design iteration: http://www.freewear.org/images/release_candidates/Wine_final.png The only difference between versions A and B is in the eyes. Let us know which one you like better, and any kind of comment or suggestion is welcomed. After the design is approved, in one week we make have the T-shirt available in the store. Cheers, Ismael
Re: dplayx: Enclose NS_GetOtherMagic() in #if 0 since it's only used from code enclosed in #if 0.
NS_GetOtherMagic is probably safe to remove.
NS_GetNsMagic was only used in some hackish code that tried to reverse
engineer dplay protocol. I removed that code that used it, but don't
know if it'll be needed in a future.
On Wed, May 13, 2009 at 12:10 PM, Kai Blin kai.b...@gmail.com wrote:
On Wednesday 13 May 2009 10:38:08 Francois Gouget wrote:
Personally I think we should just kill all that unused code and be done with
it. Currently the only way to get a working dplay implementation is to use
the native dlls anyway. Our version has been bit-rotting for quite some time.
Cheers,
Kai
1) Remove this dead code altogether...
2) Replace the #if 0 with an if (0) so that the function is 'used'
3) Fix the code so it has neither #if 0 nor if (0) (which might be the
same as option 1, I have no idea)
dlls/dplayx/name_server.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/dlls/dplayx/name_server.c b/dlls/dplayx/name_server.c
index e6a6b62..a503270 100644
--- a/dlls/dplayx/name_server.c
+++ b/dlls/dplayx/name_server.c
@@ -183,6 +183,7 @@ DWORD NS_GetNsMagic( LPVOID lpNSInfo )
return lpHdrInfo[1];
}
+#if 0
/* Get the magic number associated with the non NS end */
DWORD NS_GetOtherMagic( LPVOID lpNSInfo )
{
@@ -190,6 +191,7 @@ DWORD NS_GetOtherMagic( LPVOID lpNSInfo )
return ((LPDWORD)lpCache-lpLocalAddrHdr)[1];
}
+#endif
void NS_SetLocalAddr( LPVOID lpNSInfo, LPCVOID lpHdr, DWORD dwHdrSize )
{
--
Kai Blin
WorldForge developer http://www.worldforge.org/
Wine developer http://wiki.winehq.org/KaiBlin
Samba team member http://www.samba.org/samba/team/
--
Will code for cotton.
Re: wine shirts
Hi there We didn't forget about the T-shirt, even if there haven't been news for a while :) Our artist has been very busy lately with other projects, but hopefully she can start polishing the design in one or two weeks... Cheers Ismael On Fri, Feb 27, 2009 at 4:07 PM, Dan Kegel d...@kegel.com wrote: That's a nice, simple design, but something's missing. Oddly, the original drunken penguin shirts, or ripoffs thereof, seem to still be available at http://www.ixsoft.de/software/products/CWTSHIRTDP-L.html Original artwork is at ftp://wine.codeweavers.com/pub/wine/logos/ I would kind of like a shirt that had the penguin opening the bottle on the front (text: Support the Wine Project), and the drunken penguin on the back (text: I did my part, or something like that) but I'd have to think a bit more on the text. Other old links: Martin's design: http://cross-lfs.org/~mlankhorst/t-shirt.png Ismael Barros put some effort into this before: http://www.winehq.org/pipermail/wine-devel/2008-December/070894.html but nobody could think up good text. - Dan
Re: wine shirts
On Mon, Mar 2, 2009 at 5:33 PM, Hervÿe9 Chanal chanal_he...@yahoo.fr wrote: http://img522.imageshack.us/img522/8526/wines2.jpg Heh, looks cool, too bad we can't print a design with so many gradients If you want some dumb Wine slogans to inspire you, there are some in http://spreadsheets.google.com/viewform?key=pFqhThgtfGxo3OxU8pe8ZDg
Re: Wine t-shirts?
Looks like the poll is pretty dead, and unless somebody wants to link it somewhere, I suppose we should start getting some conclussion White T-shirt wins 17-11 over white and red, but designs A and B are drawn, so our designer will choose (and she really disliked option A :P) About the slogan, there are 5 for no slogan at all and 4 for run windows applications etc, and I see the drunk penguin T-shirt more suitable with either an informal slogan or just with the project name and website. So according to the poll (if nobody else wants to vote, the poll is still open) we have cute drunk penguin D with no slogan, Wine name, www.winehq.org somewhere and white T-shirt. When we are able to get more Wine T-shirts going, we will use design A and run windows applications etc slogan. Anything else?
Re: Wine t-shirts?
Windows translation layer Run Windows applications on Linux, BSD and Mac OS X. It doesn't really sound very T-shirt-y to me. I think I'd rather wear a T-shirt with a short, witty slogan than being a walking banner :) Something like Releasing your computer of Windows since 1993 or Closing your Windows since 1993 or Putting the Win in Windows(TM) since 1993 (okay that last one was more stupid than it should be) would do the work, enough to avoid alcoholic confussions yet not that boring. If someone is that interested in what are the windows you're so interested in closing, they may ask.
Re: Wine t-shirts?
On Wed, Dec 17, 2008 at 4:49 PM, Andrew Fenn andrewf...@gmail.com wrote: Thinking about me walking down the street again I have to say I prefer Remco's suggestion the best because it describes best what wine does. I like the idea of a witty shirt however the lines suggested requires that you already understand what wine and open source is about to get the joke. Well, I personally tend to prefer T-shirts that not everybody understands, but that cause some good laughs on the chosen ones that do :P We'll select one kind of T-shirt depending on the poll, but if the store works find we should be able to offer alternative kinds of design soon. Perhaps it would be best to have a combination of a funny picture, the project name, Remco's suggestion for the text and possibly the wine logo? Just my opinion, perhaps if you're doing voting you could make up some witty and non-witty shirts and see which people like the most. I updated the poll ( http://spreadsheets.google.com/viewform?key=pFqhThgtfGxo3OxU8pe8ZDg ) with slogan options. I hope it still works for the ones who had already voted (which should answer only the last question, not all three)
Re: Wine t-shirts?
On Thu, Dec 4, 2008 at 5:47 AM, Steven Edwards winehac...@gmail.com wrote: Heh sorry for all the spam, it would help if I used gmail for everything, in the original thread Jer already gave blanket permission for Wine to that logo: fromJeremy White jwh...@winehq.org to Dan Kegel d...@kegel.com cc Wine Developers List wine-devel@winehq.org dateWed, Apr 23, 2008 at 7:40 AM subject Re: Wine t-shirts? mailing listwine-devel.winehq.org Filter messages from this mailing list http://kegel.com/wine/wine-penguin-corkscrew.png Jon Parshall is proud of his ability to draw those penguins, and we'd gladly send that artwork to anyone that wanted to make a T-shirt. Note, though, that we felt we had to (very sadly) drop the penguin; it doesn't really work for Mac users. Again sorry for the spam, I will stop with it now. -- Steven Edwards There is one thing stronger than all the armies in the world, and that is an idea whose time has come. - Victor Hugo Hi there, some preliminar desings, with two T-shirt models to choose from: http://www.freewear.org/images/release_candidates/propuesta_Wine.png http://www.freewear.org/images/release_candidates/Wine_baseball.png Which model and T-shirt do you like better? Any suggestion to improve them? Maybe in a future we can sell more than one Wine design, but right now we'd rather offer only one, to avoid logistic problems. I've created this to make voting easier: http://spreadsheets.google.com/viewform?key=pFqhThgtfGxo3OxU8pe8ZDg Cheers Ismael Barros
Re: Wine t-shirts?
On Wed, Dec 17, 2008 at 12:58 AM, Andrew Fenn andrewf...@gmail.com wrote: Shouldn't you take off the hq part of the name? Indeed, I forgot to tell the arts team that. I don't know how exactly you could do what I am about to suggest, but is there a way to make the graphics tell a little more as to what wine is? It's just that I can picture myself walking down the street in this and everyone thinking I'm some kind of alcoholic. Well, what do you expect from a program called wine? :P All the wine logos I know are wine-related. Maybe we can use them but with some kind of C source code background, but even then most people would think you are some kind of alcoholic geek. Also I think the web address should be on there somewhere. Maybe in the back, but I think that would make the T-shirt more expensive. I'll ask. On Wed, Dec 17, 2008 at 1:01 AM, Jeff Zaroyko jeffzaro...@gmail.com wrote: If the hq is taken off the name, shouldn't it then be Wine rather than WINE? Why? It's mostly written in Wine form, but it's also an acronym, so the way that looks better on the T-shirt should work fine.
Re: Wine t-shirts?
On Thu, May 8, 2008 at 10:32 PM, Maarten Lankhorst [EMAIL PROTECTED] wrote: 2008/5/8 Maarten Lankhorst [EMAIL PROTECTED]: 2008/5/8 Jon Parshall [EMAIL PROTECTED]: I've messed around with Wine Bottles for our installer artwork, and I have this version that I could gussy up for the shirts if you'd like. I like the most left bottle, can you make one in the reddish color wine uses? And put the label I created on it and get it roughly in the same size? It would be perfect for the t-shirt. Uncorked, since wine will be released. :D Hi all I'm setting up a T-shirt store specialized in free software, www.freewear.org (work still in progress, but if everything goes well it should be running in one or two weeks), and obviously want Wine T-shirts in :) I already asked julliard about permissions and donations (winehq is getting 3€ for each sold T-shirt), and now I should determine the T-shirt design itself. We'll probably start off with the classical wine logo, but we're not sure about the composition, any ideas? The website logo looks fine (without the shades, we're limited to 4 colours), but the hq is probably unnecessary for a T-shirt. Anyway, if you guys want come up with a nicer design/logo, or want to order T-shirts for some Wine/FOSS event, we're open for business. Cheers Ismael Barros
Re: Wine t-shirts?
Hum, that would be cool, I'll ask the boss but it's probably in. Does anyone have a higher resolution version? On Wed, Dec 3, 2008 at 4:29 PM, Steven Edwards [EMAIL PROTECTED] wrote: On Wed, Dec 3, 2008 at 9:50 AM, Ismael Barros² [EMAIL PROTECTED] wrote: I already asked julliard about permissions and donations (winehq is getting 3€ for each sold T-shirt), and now I should determine the T-shirt design itself. We'll probably start off with the classical wine logo, but we're not sure about the composition, any ideas? The website logo looks fine (without the shades, we're limited to 4 colours), but the hq is probably unnecessary for a T-shirt. +1 on the teeshirts if you can use the drunk Penguin on some of them now that CodeWeavers is not using it for products. http://www.ixsoft.de/software/products/CWTSHIRTDP-L.html -- Steven Edwards There is one thing stronger than all the armies in the world, and that is an idea whose time has come. - Victor Hugo
Re: Fixing the conformance tests
On 9/6/08, James Hawkins [EMAIL PROTECTED] wrote: Tests with timeouts === - dplayx:dplayx fails with a timeout on all windows servers According to Ismael, he has a few fixes that cut the time needed in half. Ismael, what's the status on this? http://winehq.org/pipermail/wine-devel/2008-August/068125.html I managed to cut the time (in 3/4, if I recall correctly) just changing a default timeout value in a function, but times were still bigger than desired (around 7 minutes for all tests, I think). Then I tried to run all tests concurrently, but it looks like dplay doesn't like concurrency too much, in my Windows XP I started seeing ~200 errors in tests that produce 0 errors when run secuentially. When running like only the 4 first tests at a time, it somtimes produced errors, sometimes didn't, and I'm afraid random results in tests are not really liked. This attempt to make the tests concurrent can be found at http://repo.or.cz/w/wine/gsoc_dplay.git I'll try to figure out how to solve this, but right now and until next week I'll be busy with exams, and even after that I won't have much time to spend on wine. If somebody wants to quickly get rid of those errors for now, it'd be okay to comment the test calls out in the START_TEST of tests/dplay.c, I'll re-enable them when I find a solution. About the errors in non-XP versions of Windows, I suppose I'll have to get and virtualize them to run the tests against them. Cheers Ismael
Re: Summer of code evaluations
On Wed, Aug 20, 2008 at 1:14 AM, Maarten Lankhorst [EMAIL PROTECTED] wrote: I would like to request from the mentors to fill in the final evaluation form and from the students to give a final write up: What went well? Did you meet the goals you set? Did you have fun? Is there anything we can do to make wines summer of code better and do you feel like you've become part of the wine community? My project didn't go bad, but I was only able to complete around 40% of what I expected to do in the begining, it turned out to be bigger than I thought. I'll try to finish it, but I probably won't be able to start working on it again until mid-september (exams, other projects abbandoned due to GSoC, etc), and then my progress will be much slower. I really enjoyed this SoC, I was able to update my skills and learn to work in a big project. Thanks to kblin and to all the people who reviewed my code and bothered to suggest improvements :) Cheers
Re: dplayx tests hanging
On 8/11/08, Ismael Barros [EMAIL PROTECTED] wrote: Actually the thread stuff is a very good idea, I'll take a look. I tried launching each tests in a new thread, but a lot of tests failures arise, sometimes even with segfaults or deadlocks. Looks like the original implementation of dplay doesn't like concurrency too much. The good news are that playing with the timeout value of EnumSessions I already managed to cut the time of some tests in half. -- ...yet even then, we ran like the wind, whilst our laughter echoed, under cerulean skies...
Re: dplayx tests hanging
On 8/11/08, Kai Blin [EMAIL PROTECTED] wrote: On Sunday 10 August 2008 18:34:02 Dan Kegel wrote: http://test.winehq.org/data/tests/dplayx:dplayx.html Since your commits on the 4th of Aug, the dplayx tests have been hanging. This is getting in the way of my regression testing. I've toyed with the tests a little and the following tests took over 30s to finish: test_Open() 49s test_EnumSessions() 180s test_CreatePlayer() 32s test_GetPlayerAccount() 63s, also 1 test failure on XP A bunch of other tests took 20s, and test_GetPlayerAddress() had 7 failures. Incidently, all tests that took over 20 seconds took around 23 seconds, which smells like some fixed timeout. That's probably the main problem, as there's no network latency (all the messages are sent to localhost) or cpu intensive operation. Actually the thread stuff is a very good idea, I'll take a look. I already tried changing the default timeout for EnumSessions, and on XP the whole test set changed from 7 to 4 minutes, but with a lot of failures. And thanks for the info, what did you use to time the tests? Also, could you send me the log of the failed tests? They all work on my XP.
Re: dplayx tests hanging
On 8/11/08, Kai Blin [EMAIL PROTECTED] wrote: On Monday 11 August 2008 18:17:58 Ismael Barros wrote: That's probably the main problem, as there's no network latency (all the messages are sent to localhost) or cpu intensive operation. Actually the thread stuff is a very good idea, I'll take a look. I already tried changing the default timeout for EnumSessions, and on XP the whole test set changed from 7 to 4 minutes, but with a lot of failures. What kind of failures? A lot of tests failed for no obvious reason. I can start trying bigger timeouts to see if all the tests start working again, but it's difficult to make sure that'll work on every system... And thanks for the info, what did you use to time the tests? Also, could you send me the log of the failed tests? They all work on my XP. Henri pointed me at timeit from the Win2k3 ressource kit at http://www.microsoft.com/downloads/details.aspx?familyid=9d467a69-57ff-4ae7-96ee-b18c4790cffddisplaylang=en I just ran one test at a time and noted the time the test ran. Attached is the Godsend, I've been looking for a similar app but fallbacked to my phone's chronometer. output of the unmodified dplayx tests, running on a vanilla WinXP ProfessionalN SP2 box, without any extra dx runtime installed. My system is the same but with debug dx libraries. I'll probably remove those tests, they are too intensive (checking the size of received messages and suchlike), and it's funny that the GetPlayerAccount() takes a minute, it's actually a stub because that function only works with secure sessions and I still didn't manage to get them working (and they probably aren't even important for the 99% of the games)
Re: dplayx tests hanging
On 8/10/08, Dan Kegel [EMAIL PROTECTED] wrote: Hi Ismael, have a look at http://test.winehq.org/data/tests/dplayx:dplayx.html Since your commits on the 4th of Aug, the dplayx tests have been hanging. This is getting in the way of my regression testing. Can you have a look? Thanks, Dan Are they really hanging or taking too long? Dplay is quite slow and the tests are intensive so they take a lot of time. They take 7 minutes in my XP and some seconds in wine. Anyway I'll try to see if there's any way of speeding them up, I'm sorry for the hassle. Maybe the best solution is to remove dplay tests until I've finished implementing the library, as right now they are useful only for me, and a problem for the rest. Cheers Ismael -- ...yet even then, we ran like the wind, whilst our laughter echoed, under cerulean skies...
Re: [dplayx 01/12] Tests for CreateGroup
On 8/5/08, Paul Vriens [EMAIL PROTECTED] wrote: Ismael Barros wrote: Tests for CreateGroup Fixed the implementation of CreateGroup and CreateGroupInGroup so that it doesn't segfault when the service provider is not initialized --- dlls/dplayx/dplay.c| 10 ++ dlls/dplayx/tests/dplayx.c | 340 2 files changed, 350 insertions(+), 0 deletions(-) Hi Ismael, On what Windows version are you running your tests for verification? Is there anything particular you are doing to speed up the tests on your box? I'm running them in XP SP2, in vmware, and I don't have ane way to speed them up, dplay is quite slow setting up connections. test.winehq.org already shows timeouts for all boxes that are running W2K or higher. This new patch serie will even add more tests but I'm afaid we will not see any output from it on the mentioned website. All the tests work on my machine, I'll have to check how to stop them failing in Win98 and NT4, their behavior seems completely different... Why do timeouts appear? Tests take too long? Too long without output to stdout? Is there any way to solve it without rewriting everything? -- Cheers, Paul. -- ...yet even then, we ran like the wind, whilst our laughter echoed, under cerulean skies...
Re: [dplayx] New library dpwsockx, needed by dplayx
On 7/13/08, Alexandre Julliard [EMAIL PROTECTED] wrote: Michael Karcher [EMAIL PROTECTED] writes: Hello Alexandre, this thread is about an include file specifying the interface between dplayx.dll and the DirectPlay service providers. As it is not in the Windows Platform SDK, James Hawkins explained that it may not go into include. This file is shared between dplayx and the service providers, currently two copies exist (if the patch that started the discussion gets applied), and if a modem provider gets written, three copies exist. What do you think would be the appropriate location of that file? If it's part of some DirectPlay SDK then it can go in include, otherwise it will probably have to go in include/wine. It's not part of any official DirectPlay SDK, but I suppose it's part of the documentation Microsoft provides if you ask them for the interfaces needed to develop a new service provider. However I can't confirm this yet (they agreed to provide me the documentation, but I've got no news from them since then), so include/wine is probably the place then. What would be the exact path? include/wine/dplaysp.h? include/wine/dplay/dplaysp.h? Cheers Ismael
Re: [dplayx] New library dpwsockx, needed by dplayx
Nop, actually I also thought the best solution would be to move that file to include, because in fact it's just copied from dlls/dplayx, but AFAIK wine doesn't want to put in include files that are not originally present in Windows. However, if it's okay I'll move it, much better. I will also have to implement a stub modem provider, so I would have to copy that dplaysp.h again to dlls/dpmodemx, and any change in those definitions would have to be done three times... On 7/12/08, Michael Karcher [EMAIL PROTECTED] wrote: Am Samstag, den 12.07.2008, 18:46 +0300 schrieb Ismael Barros: I'm sorry, that patch lacked a header file, this is the fixed patch. I would put the dplaysp.h file into include, not dlls/dpwsockx. It contains definitions for all DirectPlay service providers, so it might be useful if a modem provider or a direct serial connection provider will be implemented. Is there a reason not to do so? Regards, Michael Karcher -- ...yet even then, we ran like the wind, whilst our laughter echoed, under cerulean skies...
Re: [dplayx] New library dpwsockx, needed by dplayx
On 7/12/08, James Hawkins [EMAIL PROTECTED] wrote: On Sat, Jul 12, 2008 at 12:25 PM, Ismael Barros [EMAIL PROTECTED] wrote: Nop, actually I also thought the best solution would be to move that file to include, because in fact it's just copied from dlls/dplayx, but AFAIK wine doesn't want to put in include files that are not originally present in Windows. However, if it's okay I'll move it, much better. I will also have to implement a stub modem provider, so I would have to copy that dplaysp.h again to dlls/dpmodemx, and any change in those definitions would have to be done three times... If it's not in the SDK, it can't go into include. Also, please bottom-post on this mailing list. As the Peter Hunnisett (author of dplaysp.h) told me, you have to ask Microsoft for the documentation and interface to create a new service provider, as this information was not made public because they never thought it were to become useful for anyone. I suppose this file was extracted from that documentation, but I can't confirm it yet.
Re: [dplayx 02/29] Tests for DirectPlayCreate
On 7/11/08, Michael Karcher [EMAIL PROTECTED] wrote: Am Donnerstag, den 10.07.2008, 22:04 +0300 schrieb Ismael Barros: I've been checking how this is done in other tests, and it's particularly interesting how they do it in ddraw/tests/refcount.c:81: hr = IDirectDraw7_CreatePalette(DDraw7, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, palette, (void *) 0xdeadbeef); I don't think this line is good style, but I still like it more than your program, as this will definitely crash if the pointer is derefenced, while your variant makes the function derefencing the pointer accessing uninitialized data. If it's not that bad, would it be okay to leave it with the deadbeef solution, with a TODO comment to do it the right way later? Right now I would like to focus on other functionality (correct networking etc), and maybe leave that kind of details for after GSoC. From a pragmatic point of view, I doubt many games make use of theese COM features, while they still can't properly enumerate service providers. This is the only example in the current code. I could either do it that way (not correct, but simple), or use some IUnknown interface. I tried to get the IUnknown of the classes I have access to, DirectPlay or DirectPlayLobby, but the current implementation is not able to return an interface for IID_IUnknown (it just fails with E_NOINTERFACE), Thats bad. Every COM object must return a valid interface pointer for IID_IUnknown. And even worse: The IID_IUnknown pointer *has* to be stable. So if you do the following (pretend that CreateFooBar(REFIID, DWORD, IUnknown*) is a factory function: IFooBar *fb1, *fb2; IUnknown *unk1, *unk2; fb1 = CreateFooBar(IID_IFooBar, 0, NULL); IFooBar_QueryInterface(fb1, IID_IUnknown, unk1); IFooBar_QueryInterface(fb1, IID_IFooBar, (IUnknown*)fb2); IFooBar_Release(fb1); IFooBar_QueryInterface(fb2, IID_IUnknown, unk2); IFooBar_Release(fb2); ok(unk1 == unk2, COM violation: IUnknown interface must be stable!); IUnknown_Release(unk1); IUnknown_Release(unk2); The COM specification guarantees the test to succeed, independent of what type IFooBar has! DirectPlay currently has no permanent COM object, but creates a new object on the fly in QueryInterface; this is needed to support the different vtables. Great input, I'll bear it in mind :)
Re: [dplayx 02/29] Tests for DirectPlayCreate
And last but not least: As I see it, a static inline would do the job too, and is always preferrable to macros. The main reason of using ditry macros instead of functions was to preserve the __LINE__ number, otherwise the debug information provided is pretty much useless. Yes, you are right of course. I didn't think of __LINE__ (also James noted the same thing). Now you get to the point where a precedence case for identifiers with an underscore are used: shell32/tests/shlexec.c. They have inline functions, called _okFooBar which take file and line as parameter, and have a macro okFooBar which calls the _okFooBar method, so the test case writer does not get to see the identifier starting with an underscore. Thats how I feel about them: They are internal identifiers, telling you if you use this directly, you better know very well what you are doing; most probably there is a safer or easier way to accomplish the goal. In this case, the _okFooBar function is meant just for the easier okFooBar macro. Sounds fine, I'll change it. +heap = HeapCreate( 0, 0, 0 ); Do you have any reason to create your own heap? Each windows process already comes equipped with a standard heap you can obtain using the API function GetProcessHeap, so this line: +IUnknown* pUnk = HeapAlloc( heap, 0, sizeof(LPDWORD) ); would then look like: +IUnknown* pUnk = HeapAlloc( GetProcessHeap(), 0, sizeof(LPDWORD) ); That's what I used in the begining, but I've got some HeapAllocs that I don't free. OK. That's a point. I didn't expect it, though. I would like a comment in there along /* Create an extra heap to throw away garbage after tests */ No problem. This is intended, as freeing that memory would make the code unnecessarily complex, I can't use static local variables (I use the result of the function inside the same ok() or trace()), and loosing memory is always undesirable but not critical in a test case. You can use static variables, if you go along the lines of of get_temp_buffer in libs/wine/debug.c, where a static local array is cyclical indexed. This might not be a good idea, though, depending on how long the result has to stay alive. Hm, yeah, I thought about a cyclic buffer but wanted to keep the code simple. However this heap thing is creating some skepticism, so I'll go that way. Another point is that this line is totally wrong anyway. An IUnknown* is a pointer to an COM object, which starts with a vtable pointer. You let pUnk point to uninitialized memory. While it doesn't matter in this case (as DirectPlayCreate will never access what the pointer points to), it is really bad style. I understand the intention to test that aggregation is not supported for DirectPlay, but you still should keep the contract of the creation function that pUnk either is NULL or points to a valid COM object. A basic COM object is just a pointer to a vtable that contains a QueryInterface method that returns this for IUnknown and fails in every other case, an AddRef method that returns 2 and does nothing, and a Release method that returns 1 and does nothing else. I just wrote is to check the return value in case of pUnk != NULL, but there would be no problem in either removing that test (it's probably unneeded) or doing it the right way. A test case is always good, if it does not take too much execution time. These tests with NULL pointers seemed strange to me when I first encountered, as sometimes NULL pointers are passed into functions where they are not useful or even forbidden by the documented contract. But as wine is all about fulfilling the same contract as the windows implementation which might deviate from MSDN, the test case serves as documentation of Windows behavior, which is good. But in my opinion, testing of the reaction to contract violation should not go into passing somehow valid-looking pointers, that do not point to anything sensible. And the main point was, that I really don't get why it must be a heap pointer. The an LPDWORD on the stack would do as well as allocating sizeof(LPDWORD) area. I've been checking how this is done in other tests, and it's particularly interesting how they do it in ddraw/tests/refcount.c:81: hr = IDirectDraw7_CreatePalette(DDraw7, DDPCAPS_ALLOW256 | DDPCAPS_8BIT, Table, palette, (void *) 0xdeadbeef); This is the only example in the current code. I could either do it that way (not correct, but simple), or use some IUnknown interface. I tried to get the IUnknown of the classes I have access to, DirectPlay or DirectPlayLobby, but the current implementation is not able to return an interface for IID_IUnknown (it just fails with E_NOINTERFACE), so I would have to fix that too (and I'm not very sure about how to do it, without breaking anything. Would returning any of the existing interfaces that have CreateInstance, AddRef and Release, be correct? This seems to be done a lot in other dlls)
Re: [dplayx 02/29] Tests for DirectPlayCreate
Thanks for the review :) On 7/9/08, Michael Karcher [EMAIL PROTECTED] wrote: Hello Ismael, reviewing this patch shows some issues. +#define _okHR(expected, result) \ +ok( (expected) == (result), \ +expected=%d(%s) got=%d(%s)\n, \ +HRESULT_CODE(expected), dpResult2str(expected), \ +HRESULT_CODE(result), dpResult2str(result) ); I dislike the order of the parameters and the name. You would write ok( hr == DP_OK, Something failed with %x\n, hr); In this line, hr is before DP_OK. So as an analogy, I also want to write _okHR( hr, DD_OK ); Depending on the situation I write ok( DP_OK == hr ) instead of ok( hr == DP_OK, ... ), specially if instead of hr there is a DirectPlay call or some complex expression, in order to improve readability. Regarding this second case, I decided to use the (expected, result) order, but it's true that sometimes it can be confusing. Why do you start the macro name with an underscore? Because _okHR compares HRESULT codes, but there's also _ok to compare integers, or _okFlags to compare flags (the main difference being how to print info about the expected and actual values), or _okStr to compare strings. And last but not least: As I see it, a static inline would do the job too, and is always preferrable to macros. The main reason of using ditry macros instead of functions was to preserve the __LINE__ number, otherwise the debug information provided is pretty much useless. +heap = HeapCreate( 0, 0, 0 ); Do you have any reason to create your own heap? Each windows process already comes equipped with a standard heap you can obtain using the API function GetProcessHeap, so this line: +IUnknown* pUnk = HeapAlloc( heap, 0, sizeof(LPDWORD) ); would then look like: +IUnknown* pUnk = HeapAlloc( GetProcessHeap(), 0, sizeof(LPDWORD) ); That's what I used in the begining, but I've got some HeapAllocs that I don't free. This is intended, as freeing that memory would make the code unnecessarily complex, I can't use static local variables (I use the result of the function inside the same ok() or trace()), and loosing memory is always undesirable but not critical in a test case. The main problem was to get yelled when valgrind complains about the memory leak, and Kai Blin advised me to use my own heap to avoid valgrind complains. Anyway is this is too dirty I'll have no problem in doing it correctly. Another point is that this line is totally wrong anyway. An IUnknown* is a pointer to an COM object, which starts with a vtable pointer. You let pUnk point to uninitialized memory. While it doesn't matter in this case (as DirectPlayCreate will never access what the pointer points to), it is really bad style. I understand the intention to test that aggregation is not supported for DirectPlay, but you still should keep the contract of the creation function that pUnk either is NULL or points to a valid COM object. A basic COM object is just a pointer to a vtable that contains a QueryInterface method that returns this for IUnknown and fails in every other case, an AddRef method that returns 2 and does nothing, and a Release method that returns 1 and does nothing else. I just wrote is to check the return value in case of pUnk != NULL, but there would be no problem in either removing that test (it's probably unneeded) or doing it the right way. +/* pUnk==NULL, pDP!=NULL */ +hr = DirectPlayCreate( NULL, pDP, NULL ); +_okHR( DPERR_INVALIDPARAMS, hr ); +hr = DirectPlayCreate( (LPGUID) _GUID_NULL, pDP, NULL ); +_okHR( DP_OK, hr ); +hr = DirectPlayCreate( (LPGUID) DPSPGUID_TCPIP, pDP, NULL ); +todo_wine _okHR( DP_OK, hr ); Here you are leaking DirectPlay objects. If DirectPlayCreate worked, you have to release the object before you overwrite pDP (by passing it to DirectPlayCreate again) using IDirectPlay_Release(pDP). Roger Regards, Michael Karcher
