Re: winspool.drv: make sure to use return values (LLVM/Clang)
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=8921 Your paranoid android. === WINEBUILD (build) === Can't determine base name
Wine API documentation questions.
I've been going through the Wine API documentation and there seem to be some things that I think should be changed: - There are references to Wine documents that do not include links to the pages. The links should be included. - Some 'implementation' sections claim that the API is not declared but there are declarations, just not in headers that 'c2man' is looking at. It should also look in any sub-directories (except for 'tests') of the '-I' directories it is told about for additional headers. (I have very little skill with Perl at present, so someone else might be able to add this much faster than I will be able to do it.) - There are quite a few APIs implemented but without standard documentation. All accessible APIs should have enough documentation so that there is a page for it, even if it is only a synopsis. - Mangled interface names only appear in their mangled form. The documentation should include the demangled name as well. Should the demangled names also appear in the indexes? - The 'synopsis' does not include needed '#include' directives. This may need to be fixed in each API description rather than trying to get 'c2man' to generate it. - The name of the file containing the implementation is included in each 'implementation' section and includes a link to the appropriate repository source entry. The name of the appropriate header is also included, but does not include a link. A link should be included. Max
Killing WineAPI project on SourceForge.
No feedback and apparently no interest in the WineAPI project on SourceForge. Understandable since there is now a WineAPI on the regular Wine Wiki. Unless someone objects, I'm going to ask that it be shutdown on SourceForge. Max
Re: kernel32/process: Add stderr redirection for native linux programs started from windows program
On 02/04/2011 06:25 PM, Austin English wrote: On Fri, Feb 4, 2011 at 18:22, Igor Egorov wrote: This is my first patch, not sure if I done all right. I didn't look at the actual change, but: mode change 100644 => 100755 dlls/kernel32/process.c doesn't seem necessary to me. Yes, thanks, I resent the patch
Re: kernel32/process: Add stderr redirection for native linux programs started from windows program
On Fri, Feb 4, 2011 at 18:22, Igor Egorov wrote: > This is my first patch, not sure if I done all right. I didn't look at the actual change, but: > mode change 100644 => 100755 dlls/kernel32/process.c doesn't seem necessary to me. -- -Austin
Re: gdi32/tests: make sure to use return value (LLVM/Clang)
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=8891 Your paranoid android. === WVISTAADM (32 bit font) === Failure running script in VM: The specified guest user must be logged in interactively to perform this operation
Including winegcc-built shared library in Debian binary package
I've got a Debian binary package configured. A wrapper script is installed in /usr/bin that invokes wine on the winegcc-built shared library that implements the program. The shared library is installed in /usr/lib and then ldconfig is run during the installation process. The shared library build product seems to be a special case because it is built by winegcc and referenced on the wine command line when run. I've got questions about this even though the product works OK as packaged. Is there a more appropriate installation directory than /usr/lib for the shared library (although lintian complains if I don't put it in /usr/lib)? Does ldconfig need to be run after installing/uninstalling the shared library (other than to satisfy lintian complaints if ldconfig isn't run from postinst/postrm)? Is there any reason to bother with version numbering the shared library (other than using one version number for all library releases just to keep lintian happy by having the version number) since the library is prepared by winegcc and will only be loaded by wine? To look for answers, I've done the following: * searched the web * searched the WineHQ web site and read the FAQ there * scanned the subject lines of a couple of months of this mailing list * read http://wiki.winehq.org/Packaging * used https://wiki.ubuntu.com/PackagingGuide/Complete for some packaging instructions * read http://www.bytehold.com/index.php/projects/176-debian-binary-package-and-repository-howto for general instructions in setting up a Debian binary package * read http://www.pathname.com/fhs/ for recommendations on locating files URLs to relevant resources would be welcome. -- Robert McDonald BitTorrent, Inc.
Re: Pulling Patch
James McKenzie wrote: > Actually, the latest patch is what I don't want reused. This one? http://www.winehq.org/pipermail/wine-patches/2011-January/098431.html Why don't you want it reused? > And no, you don't put it in the LGPL until it is committed, That's interesting. I always thought it was LGPL'd as soon as it was sent to the patches list... maybe we should add a note to the mailing list description clarifying this. > Besides, I've been a big enough pain that my existence here is > unwarranted and unneeded. Sounds like you're sore from beating your head against the wall trying to get patches in. Sorry to hear it. So long! - Dan
Re: Pulling Patch
On 2/4/11 1:18 PM, Dan Kegel wrote: James McKenzie wrote: Since my Mac is dying I have decided to return to the Windows world. Please pull any and all patches. I have envoked the right to copyright and none of my code can or will be used in Wine. Sorry to see you go, but... why would you want to prohibit people from using your patches? (I only see two typo changes committed: http://source.winehq.org/git/wine.git/?a=search&h=HEAD&st=author&s=James+McKenzie&sr=1 Presumably you don't mean those, since they were trivial comment changes.) I guess you mean patches like http://www.winehq.org/pipermail/wine-patches/2010-June/089537.html When you sent those to wine-patches, weren't you placing them under the LGPL? Actually, the latest patch is what I don't want reused. And no, you don't put it in the LGPL until it is committed, which I don't expect AJ to do anyway. However, I'm moving in a different direction since my Mac needs more repairs than I'm willing to spend money on. Besides, I've been a big enough pain that my existence here is unwarranted and unneeded. James McKenzie
Re: Tray icon not showing up if wine was run previously with DISPLAY unset?
Dan Kegel writes: > I guess I could do "sleep 5" but that's an awful kludge. > Probably I'll go back to letting the ugly virtual desktop pop up. > So, what's the proper way to run one app under wine *without* X > while letting other apps run wine *with* X? You can't, but winepath shouldn't be popping up a virtual desktop, that's a bug. -- Alexandre Julliard julli...@winehq.org
re: Pulling Patch
James McKenzie wrote: > Since my Mac is dying I have decided to return to the Windows world. > > Please pull any and all patches. I have envoked the right to copyright > and none of my code can or will be used in Wine. Sorry to see you go, but... why would you want to prohibit people from using your patches? (I only see two typo changes committed: http://source.winehq.org/git/wine.git/?a=search&h=HEAD&st=author&s=James+McKenzie&sr=1 Presumably you don't mean those, since they were trivial comment changes.) I guess you mean patches like http://www.winehq.org/pipermail/wine-patches/2010-June/089537.html When you sent those to wine-patches, weren't you placing them under the LGPL? - Dan
Tray icon not showing up if wine was run previously with DISPLAY unset?
So, I've got this shell script that does something like h=`wine winepath -w $HOME` ... wine autohotkey run-setup.ahk Works great. However, if the user has selected to run wine in a virtual desktop, running winepath will start up and then shut down the virtual desktop, which is visually annoying. Second try: unset DISPLAY when running winepath, e.g. h=`DISPLAY= wine winepath -w $HOME` ... wine autohotkey run-setup.ahk Works great; now winepath doesn't pop up the desktop. Except now autohotkey's tray icon doesn't show up at all, which makes it hard to debug the autohotkey script. (The setup that autohotkey runs can access X just fine, so the problem seems confined to the tray icon.) Is the lack of tray icon a wine bug, or a user error? Third try: wait for wineserver to exit before starting autohotkey h=`DISPLAY= wine winepath -w $HOME` ... wineserver -w wine autohotkey run-setup.ahk but that's slower, and will hang if the user is running a wine app. I guess I could do "sleep 5" but that's an awful kludge. Probably I'll go back to letting the ugly virtual desktop pop up. So, what's the proper way to run one app under wine *without* X while letting other apps run wine *with* X? - Dan
Re: loader: Build the preloader for ARM
Den 2011-02-04 20:36 skrev Peter Rosin:
> Den 2011-02-04 17:41 skrev André Hentschel:
>> % and / both lead to external dependencies here...
>> maybe the euclidean algorithm helps, because -Os would need ugly changes to
>> configure i guess.
>>
>
> Finding gcd is a bit over the top...
>
> How about something like the following?
>
> Cheers,
> Peter
>
> unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
> {
> #ifdef __ARM_EABI__
> unsigned int r = 0;
> int i;
>
> for (i = sizeof(a) * 8 - 1; i >= 0; --i) {
> r <<= 1;
> if (a & (1U << i))
> r |= 1;
> if (r > b)
Ooops, bug. Should be "if (r >= b)"
> r -= b;
> }
>
> return r;
> ...
>
>
Cheers,
Peter
Re: loader: Build the preloader for ARM
Den 2011-02-04 17:41 skrev André Hentschel:
> Am 04.02.2011 17:26, schrieb Marcus Meissner:
>> On Fri, Feb 04, 2011 at 04:51:46PM +0100, André Hentschel wrote:
>>> Am 04.02.2011 16:31, schrieb Alexandre Julliard:
André Hentschel writes:
> +unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
> +{
> +#ifdef __ARM_EABI__
> +unsigned int i;
> +if(b > a) return a;
> +for(i = 0; i * b < a + b; i++)
> +if(i * b > a - b) return (a - i * b);
> +return 0;
That's very inefficient, there are better ways...
>>>
>>> Thanks for having a look at it.
>>> The Problem is that i also can't use '/', otherwise it wants to link to
>>> __aeabi_uidiv .
>>> That leads to some Problems, so i'll try to find some better math solution
>>> and if that fails i maybe try it the assembler way...
>>
>> modulo would be "%" btw,
>>
>> return a%b;
>>
>> but this might generate external functions too... hmm, try -Os as option?
>>
>> Ciao, Marcus
>
> % and / both lead to external dependencies here...
> maybe the euclidean algorithm helps, because -Os would need ugly changes to
> configure i guess.
>
Finding gcd is a bit over the top...
How about something like the following?
Cheers,
Peter
unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
{
#ifdef __ARM_EABI__
unsigned int r = 0;
int i;
for (i = sizeof(a) * 8 - 1; i >= 0; --i) {
r <<= 1;
if (a & (1U << i))
r |= 1;
if (r > b)
r -= b;
}
return r;
...
Re: the patches around ok()
Am 04.02.2011 20:00, schrieb Joris Huizer: > Hello Henri Verbeet, and André Hentschel > > I wasn't (actively) on the mailing list, that's why I couldn't reply directly > to your messages. > I will try and make sure I have better titles for such patches in future! > > Regards, > Joris > Great, but you should also resend them this time. I guess something like that would be ok: "imagehlp/tests/integrity.c: Don't test function directly when reporting GetLastError() (resend)" -- Best Regards, André Hentschel
the patches around ok()
Hello Henri Verbeet, and André Hentschel I wasn't (actively) on the mailing list, that's why I couldn't reply directly to your messages. I will try and make sure I have better titles for such patches in future! Regards, Joris
Pulling Patch
Since my Mac is dying I have decided to return to the Windows world. Please pull any and all patches. I have envoked the right to copyright and none of my code can or will be used in Wine. Thank you. James McKenzie
[PATCH] winegcc - do not create .exe.so
Hello, I created patch which implement correct _start section in ELF shared libraries/binaries which generate winegcc. Into _start section this patch add calling execve syscall which start wine with all arguments. So it is not needed to generate shell wrapper and .exe.so binary. This replace code on: http://wiki.winehq.org/Winelib_binaries_as_ELF_executables_not_.exe.so $ winegcc app.c --> generate only a.exe (shared object ELF binary), no aditional a.exe.so $ ./a.exe --> start binary correctly without shell wrapper -- Pali Rohár pali.ro...@gmail.com winegcc.patch Description: Binary data
Re: kernel32/tests/thread.c: Don't call in ok()
Am 04.02.2011 17:24, schrieb Henri Verbeet: > On 4 February 2011 17:15, Joris Huizer wrote: >> Subject: kernel32/tests/thread.c: Don't call in ok() > Why not? > > Because of the GetLastError() calls. But on the other hand Joris really should mention that in the subject. Joris, please have a look at my older patches like 85386c2c6eaed6ea89eefef6c8948169323bbc28 -- Best Regards, André Hentschel
Re: loader: Build the preloader for ARM
Am 04.02.2011 17:26, schrieb Marcus Meissner:
> On Fri, Feb 04, 2011 at 04:51:46PM +0100, André Hentschel wrote:
>> Am 04.02.2011 16:31, schrieb Alexandre Julliard:
>>> André Hentschel writes:
>>>
+unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
+{
+#ifdef __ARM_EABI__
+unsigned int i;
+if(b > a) return a;
+for(i = 0; i * b < a + b; i++)
+if(i * b > a - b) return (a - i * b);
+return 0;
>>>
>>> That's very inefficient, there are better ways...
>>>
>>
>> Thanks for having a look at it.
>> The Problem is that i also can't use '/', otherwise it wants to link to
>> __aeabi_uidiv .
>> That leads to some Problems, so i'll try to find some better math solution
>> and if that fails i maybe try it the assembler way...
>
> modulo would be "%" btw,
>
> return a%b;
>
> but this might generate external functions too... hmm, try -Os as option?
>
> Ciao, Marcus
% and / both lead to external dependencies here...
maybe the euclidean algorithm helps, because -Os would need ugly changes to
configure i guess.
--
Best Regards, André Hentschel
Re: loader: Build the preloader for ARM
On Fri, Feb 04, 2011 at 04:51:46PM +0100, André Hentschel wrote:
> Am 04.02.2011 16:31, schrieb Alexandre Julliard:
> > André Hentschel writes:
> >
> >> +unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
> >> +{
> >> +#ifdef __ARM_EABI__
> >> +unsigned int i;
> >> +if(b > a) return a;
> >> +for(i = 0; i * b < a + b; i++)
> >> +if(i * b > a - b) return (a - i * b);
> >> +return 0;
> >
> > That's very inefficient, there are better ways...
> >
>
> Thanks for having a look at it.
> The Problem is that i also can't use '/', otherwise it wants to link to
> __aeabi_uidiv .
> That leads to some Problems, so i'll try to find some better math solution
> and if that fails i maybe try it the assembler way...
modulo would be "%" btw,
return a%b;
but this might generate external functions too... hmm, try -Os as option?
Ciao, Marcus
Re: kernel32/tests/thread.c: Don't call in ok()
On 4 February 2011 17:15, Joris Huizer wrote: > Subject: kernel32/tests/thread.c: Don't call in ok() Why not?
Re: Try to Implement my first Stub function - AbortPrinter().
On 4 February 2011 15:30, Charles Davis wrote: > Probably. Every UNIX I've seen uses CUPS (short for Common UNIX Printing CUPS is probably pretty common these days, but LPD has been around for a long time. If it's really CUPS specific it should check for CUPS though, not OS X. Also note that the code now has unused variables in the #else case.
Re: msvcr90: Fixed a typo
Am 04.02.2011 16:51, schrieb Nikolay Sivov:
> On 2/4/2011 18:33, André Hentschel wrote:
>> ---
>> dlls/msvcr90/msvcr90.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/dlls/msvcr90/msvcr90.c b/dlls/msvcr90/msvcr90.c
>> index 2c811cb..d2a61dd 100644
>> --- a/dlls/msvcr90/msvcr90.c
>> +++ b/dlls/msvcr90/msvcr90.c
>> @@ -315,7 +315,7 @@ const char * __thiscall
>> MSVCRT_type_info_name_internal_method(type_info * _this,
>> if (!_this->name)
>> {
>> /* Create and set the demangled name */
>> - /* Nota: mangled name in type_info struct always start with a '.',
>> while
>> + /* Note: mangled name in type_info struct always start with a '.',
>> while
>> * it isn't valid for mangled name.
>> * Is this '.' really part of the mangled name, or has it some other
>> meaning ?
>> */
> Maybe 'starts with' ?
True...
--
Best Regards, André Hentschel
Re: loader: Build the preloader for ARM
Am 04.02.2011 16:31, schrieb Alexandre Julliard:
> André Hentschel writes:
>
>> +unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
>> +{
>> +#ifdef __ARM_EABI__
>> +unsigned int i;
>> +if(b > a) return a;
>> +for(i = 0; i * b < a + b; i++)
>> +if(i * b > a - b) return (a - i * b);
>> +return 0;
>
> That's very inefficient, there are better ways...
>
Thanks for having a look at it.
The Problem is that i also can't use '/', otherwise it wants to link to
__aeabi_uidiv .
That leads to some Problems, so i'll try to find some better math solution and
if that fails i maybe try it the assembler way...
--
Best Regards, André Hentschel
Re: msvcr90: Fixed a typo
On 2/4/2011 18:33, André Hentschel wrote:
---
dlls/msvcr90/msvcr90.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/msvcr90/msvcr90.c b/dlls/msvcr90/msvcr90.c
index 2c811cb..d2a61dd 100644
--- a/dlls/msvcr90/msvcr90.c
+++ b/dlls/msvcr90/msvcr90.c
@@ -315,7 +315,7 @@ const char * __thiscall
MSVCRT_type_info_name_internal_method(type_info * _this,
if (!_this->name)
{
/* Create and set the demangled name */
- /* Nota: mangled name in type_info struct always start with a '.', while
+ /* Note: mangled name in type_info struct always start with a '.', while
* it isn't valid for mangled name.
* Is this '.' really part of the mangled name, or has it some other
meaning ?
*/
Maybe 'starts with' ?
Re: loader: Build the preloader for ARM
André Hentschel writes:
> +unsigned int wld_modulo( const unsigned int a, const Elf32_Word b )
> +{
> +#ifdef __ARM_EABI__
> +unsigned int i;
> +if(b > a) return a;
> +for(i = 0; i * b < a + b; i++)
> +if(i * b > a - b) return (a - i * b);
> +return 0;
That's very inefficient, there are better ways...
--
Alexandre Julliard
julli...@winehq.org
Re: Try to Implement my first Stub function - AbortPrinter().
On 2/4/11 6:53 AM, Loïc Maury wrote: > On 03/02/11 12:02, Nikolay Sivov wrote: >> Why it's Mac specific? > In fact I don't know if I need this, but I am on Mac with CUPS, I don't > know if it the same > issue for the other platform ? Probably. Every UNIX I've seen uses CUPS (short for Common UNIX Printing System). There's also HPLIP (Hewlett-Packard Linux Imaging and Printing System), but I've never seen any programs that use that directly, and I'm not even sure if it still exists anymore :). Chip
Re: Try to Implement my first Stub function - AbortPrinter().
Hello Mr.Sivov and Wine community,
On 03/02/11 12:02, Nikolay Sivov wrote:
On 2/3/2011 13:51, Loïc Maury wrote:
Hello,
I try to implement my first stub function - *AbortPrinter()*.
But before to make a patch, I need your advice.
For what I understood, AbortPrinter(), remove the document
spool file for a printer, created by StartDocPrinter(), who indicate
that a document was spooled.
I saw that the API printer functions are in dlls/winspool.drv/info.c
I understood that there are 4 structures used, *opened_printer_t* for
manage a printer,
*jobqueue_t* for a list of job for a printer, *started_doc_t* who
manage a document,
and *job_t* for manage a job.
I am on Mac Os 10.6, with CUPS.
Here is the code, what do you think ?
Patch needs to be a diff, not a copy of a call. Some useful links
http://wiki.winehq.org/GitWine, http://wiki.winehq.org/SubmittingPatches.
OK I understand.
BOOL WINAPI AbortPrinter(HANDLE hPrinter)
{
PRINTER_INFO_5W *pi5 = NULL;
BOOL ret = FALSE;
opened_printer_t *printer;
struct list *cursor, *cursor2;
job_t *job = 0;
No need to init 'job' it seems.
Ok
DWORD jobDocId;
DWORD needed;
LPWSTR portname;
#if defined(__APPLE__)
Why it's Mac specific?
In fact I don't know if I need this, but I am on Mac with CUPS, I don't
know if it the same
issue for the other platform ?
EnterCriticalSection(&printer_handles_cs);
printer = get_opened_printer(hPrinter);
if(!printer)
{
ERR("The handle for the printer is invalid.\n");
SetLastError(ERROR_INVALID_HANDLE);
If you set last error you should add a test for it.
Ok
goto end;
}
TRACE("(%s, %s, %p, %d, %d)\n",debugstr_w(printer->name)
,debugstr_w(printer->printername)
,printer->backend_printer
,printer->queue->ref
,list_count(&printer->queue->jobs));
/* No jobs to manage. */
if(list_count(&printer->queue->jobs) == 0)
{
ERR("No job in the list.\n");
It's not an error to have no jobs.
Yes, I can remove this.
SetLastError(ERROR_SPL_NO_STARTDOC);
goto end;
}
if(printer->doc) {
TRACE("Document inside for job id : %d\n", printer->doc->job_id);
jobDocId = printer->doc->job_id;
}
else {
ERR("No document.\n");
SetLastError(ERROR_SPL_NO_STARTDOC);
goto end;
}
/* For each jobs, get the job document in the double linked list */
LIST_FOR_EACH_SAFE(cursor, cursor2, &printer->queue->jobs)
{
/* Take a job. */
job = LIST_ENTRY(cursor, job_t, entry);
TRACE("(job id : %d, filename : %s, portname : %s, document
title : %s, printer name %s)\n",job->job_id
,debugstr_w(job->filename)
,debugstr_w(job->portname)
,debugstr_w(job->document_title)
,debugstr_w(job->printer_name));
if(jobDocId == job->job_id)
{
TRACE("(hf : %p, job id : %d)\n",printer->doc->hf
,printer->doc->job_id);
/* Get portname. */
if (!job->portname)
{
GetPrinterW(hPrinter, 5, NULL, 0, &needed);
pi5 = HeapAlloc(GetProcessHeap(), 0, needed);
GetPrinterW(hPrinter, 5, (LPBYTE)pi5, needed, &needed);
portname = pi5->pPortName;
}
TRACE("portname : %s\n", debugstr_w(portname));
/* Cups Port */
if(!strncmpW(portname, CUPS_Port, strlenW(CUPS_Port)))
{
TRACE("Remove job from the list.\n");
list_remove(cursor);
CloseHandle(printer->doc->hf);
DeleteFileW(job->filename);
HeapFree(GetProcessHeap(), 0, job->document_title);
HeapFree(GetProcessHeap(), 0, job->printer_name);
HeapFree(GetProcessHeap(), 0, job->portname);
HeapFree(GetProcessHeap(), 0, job->filename);
HeapFree(GetProcessHeap(), 0, job->devmode);
HeapFree(GetProcessHeap(), 0, job);
job = 0;
What is it for?
Yes, sorry it's not necessary.
ret = TRUE;
if(pi5)
HeapFree(GetProcessHeap(), 0, pi5);
No need to check point here.
Ok I will move it after the "end:" label.
}
else
FIXME("only CUPS for now.\n");
}
}
end:
LeaveCriticalSection(&printer_handles_cs);
TRACE("return %d\n",
Re: user32: Add a test for reparenting a WS_POPUP window to a WS_CHILD parent.
Dmitry Timoshkov writes: > +SetLastError(0xdeadbeef); > +todo_wine > +ok(SetForegroundWindow(popup), "SetForegroundWindow() error %d\n", > GetLastError()); It would be interesting to test what the foreground window is actually set to. > +SetLastError(0xdeadbeef); > +ok(DestroyWindow(parent), "DestroyWindow() error %d\n", GetLastError()); Please don't add invalid uses of GetLastError again, this was cleaned up recently. -- Alexandre Julliard julli...@winehq.org
Re: user32: Add a test for reparenting a WS_POPUP window to a WS_CHILD parent.
Joris Huizer wrote: > > - ret = DestroyWindow(parent); > > - ok( ret, "DestroyWindow() error %d\n", > > GetLastError()); > > + ok(DestroyWindow(parent), "DestroyWindow() > > error %d\n", GetLastError()); > > Please don't merge the call with the ok() statement; the GetLastError() call > might be executed before the function to be tested, thus giving the old > value. (There has been some cleaning up for this issue recently) > > The same goes for a few tests you added: That's no the point of the test, and actually ok() calls are just a sinity check in this case, GetLastError() results are not interesting, since the previous calls should never fail. There are many other places which do that already, feel free to send patches to correct them. -- Dmitry.
Re: user32: Add a test for reparenting a WS_POPUP window to a WS_CHILD parent.
Hello Dmitry Timoshkov, I noticed a small issue in your patch: > > - ret = DestroyWindow(parent); > - ok( ret, "DestroyWindow() error %d\n", > GetLastError()); > + ok(DestroyWindow(parent), "DestroyWindow() > error %d\n", GetLastError()); Please don't merge the call with the ok() statement; the GetLastError() call might be executed before the function to be tested, thus giving the old value. (There has been some cleaning up for this issue recently) The same goes for a few tests you added: > + SetLastError(0xdeadbeef); > + ok(SetForegroundWindow(popup), > "SetForegroundWindow() error %d\n", GetLastError()); > + SetLastError(0xdeadbeef); > +todo_wine > + ok(SetForegroundWindow(popup), > "SetForegroundWindow() error %d\n", GetLastError()); > + > + SetLastError(0xdeadbeef); > + ok(DestroyWindow(parent), "DestroyWindow() > error %d\n", GetLastError()); > + HTH, Joris
RE: Correction to crash inside RtlCaptureStackBackTrace() + test case
> You can't use exception handlers in tests I'm not sure why not, but I'll take you word of this. Without exception handling the test cases for RtlCaptureStackBackTrace would also always crash on Win 2000/NT so that makes all of the tests useless, I think. Well, at least I learned how to (/ not to) write test cases. :) Thanks, Janne
Re: wined3d/arb_program_shader: Small optimization in state_texfactor_arbfp().
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Actually I am afraid the patch isn't quite correct. If e.g. ps constant 220 is dirty, and no others are, then highest_dirty_ps_const is 220, and pshader_const_dirty[ARB_FFP_CONST_TFACTOR] is 0. After setting the texfactor you still have to reload constant nr. ARB_FFP_CONST_TFACTOR, so you have to set it to 1 in the array. If managing the dirty constant array is expensive you could try to replace it by a bitmap where you just store 1 bit per dirty constant. That reduces the memory size(helps with the CPU cache) and you can skip 8, 16 or 32 clean constants at once in shader_arb_load_constantsF but it adds some overhead because extra operations are needed to test for single bits. > +/* TODO: this call is actually very expensive, figure out a better > way... */ > GL_EXTCALL(glProgramEnvParameter4fvARB(GL_FRAGMENT_PROGRAM_ARB, > ARB_FFP_CONST_TFACTOR, col)); There's no real alternative, except hardcoding the texfactor into the shader, and I doubt that is cheaper. Maybe local constants are cheaper, but I doubt this as well. That also means you have to reload the texfactor every time you switch between fixed function pipeline replacement shaders. Am 04.02.2011 um 10:27 schrieb Stefan Dösinger: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > > Am 03.02.2011 um 23:01 schrieb Adam Martinson: > >> >> Oddly enough this gives us an average 1.8% boost in the 3DMark06 batch size >> tests. > Impressive. > > One more thing you could investigate(but that doesn't affect this patch): Try > to figure out why this code is called in the first place. The only situation > when this should happen is if the app is using fixed function fragment > processing. I wouldn't expect that 3Dmark06 does that. > > -BEGIN PGP SIGNATURE- > Version: GnuPG/MacGPG2 v2.0.17 (Darwin) > > iQIcBAEBAgAGBQJNS8aYAAoJEN0/YqbEcdMwI1AQAJder+8LZILzdT+8tA2YiwZ+ > ZEVOi4S2voFASHUVJbYuWIXkzzVu6fQlZXw/KCyvdQaslpTWQFFc/cOMXcCVjVs5 > puJJDydjCyI8FZF7ec8dCIjCHqBH4S91kxDjRmWGifdp1Qd6h/f7o1xyhOHG62xG > XZbY1xx3DhQ7lRxRtsdqqq22MM5cOqR4mCen2YPGsd04rlwsxW6zuxZ32oBof0rO > ggzBW1F2WdDHeT6rcDK5Iv7R4zdSCtl6IXfsMaT1frTcd8Mo/9jLj7GaLMnQ/UtG > JkyQbCP59AOtEKU8nwrISn71YMRkCBHOttaIAVhr6Nq02eNmcxWomSbIsb293LzP > fQHFDPJ2aO161AimO/7Nk6DbGniUtlgRGhPhrK+YJvrAhftrtZ2gFUaN3QLJgcKA > HHhkU00NsBXOJvz8YuGFDRVmpPpCLcGM7gvMIyjYy3x3W1FWO4giNxnj6CxqHmLs > ohkAbsLEnJohhpTyVEaQKTG0Zq49s5WcvkjQbmncqKdP2D9NHmi50FI8MbJJxXAu > mMW6sA6oExn31Fnm1/pQOWCZk8hidK1hRN7f25nSRa58q7bSvNT/Gw3kk6S8T5CU > yL3r5UIb64ss4QmtXY8SPIR9IDNvmnYUX4/Av/iKpZrSRHCHzpYX7Eecp2jdV/ta > jgKpR4VaDyemyij093oW > =LXas > -END PGP SIGNATURE- > > -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJNS8nSAAoJEN0/YqbEcdMwJpQP/jAISQAlW/44UoMrUWtOio+S Hb7ut/9LDm2nBvQ6AaicehMkGj7V8+07D61/jPc/9DkYjhEDCr+D1kulXZ9p4nxs 7YLWw1Wc3Z45z+3YznUbnHD4kzQIhLntEIKdl80PBBNcHpfouNMHOO5J31ncrbzc q0wvjxYJJ80pskdzbIzQv1sO7+OrwgHVIJ8lNLW36PQuadBsOrSy6vlxO7brflB+ aG0/qIyN30M83MKzdrvYZjcck+CpPwzFdsfU+3niL+hlv5JoBuZMoxqVllLdtwDG 9fe1rOF8lDDsYXhrqZdgndKSpbWn7aY9LY0ya4YQEtTiNfkSGCf+iMVBhGr0Jy7c MGa+ElXLZKq/ZbSIWVVRXdYt3O5Znvtyta2tThxW/QK7HyK3Jq3l7P+EZgzfWlQt kjkJgVXVtPox441cvr3R8oV2M57iwBhsYWwXsusC0BCD/WKFCRb/FDwY6f2G6L2p pFkKMhhC7nIaB6CBjPailQHfMC9tlhq/vB951Ir3UVIOAUuEHrKHK9L0jercnTfd QPym37IygBdAFjNKOvPI2b8DmVFKUAkkDGdaNZSU1Z8IMCogzs9bqf6L2JFF97U3 mH1PG6/tJnsBMUfUhnA8xreg7FyqIpmn53XV14q2XFtiAWY8IRhF97p8N71oSAUJ jhUnwEL1Tw51/p6GIIu7 =dmun -END PGP SIGNATURE-
Re: wined3d/arb_program_shader: Small optimization in state_texfactor_arbfp().
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 03.02.2011 um 23:01 schrieb Adam Martinson: > > Oddly enough this gives us an average 1.8% boost in the 3DMark06 batch size > tests. Impressive. One more thing you could investigate(but that doesn't affect this patch): Try to figure out why this code is called in the first place. The only situation when this should happen is if the app is using fixed function fragment processing. I wouldn't expect that 3Dmark06 does that. -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJNS8aYAAoJEN0/YqbEcdMwI1AQAJder+8LZILzdT+8tA2YiwZ+ ZEVOi4S2voFASHUVJbYuWIXkzzVu6fQlZXw/KCyvdQaslpTWQFFc/cOMXcCVjVs5 puJJDydjCyI8FZF7ec8dCIjCHqBH4S91kxDjRmWGifdp1Qd6h/f7o1xyhOHG62xG XZbY1xx3DhQ7lRxRtsdqqq22MM5cOqR4mCen2YPGsd04rlwsxW6zuxZ32oBof0rO ggzBW1F2WdDHeT6rcDK5Iv7R4zdSCtl6IXfsMaT1frTcd8Mo/9jLj7GaLMnQ/UtG JkyQbCP59AOtEKU8nwrISn71YMRkCBHOttaIAVhr6Nq02eNmcxWomSbIsb293LzP fQHFDPJ2aO161AimO/7Nk6DbGniUtlgRGhPhrK+YJvrAhftrtZ2gFUaN3QLJgcKA HHhkU00NsBXOJvz8YuGFDRVmpPpCLcGM7gvMIyjYy3x3W1FWO4giNxnj6CxqHmLs ohkAbsLEnJohhpTyVEaQKTG0Zq49s5WcvkjQbmncqKdP2D9NHmi50FI8MbJJxXAu mMW6sA6oExn31Fnm1/pQOWCZk8hidK1hRN7f25nSRa58q7bSvNT/Gw3kk6S8T5CU yL3r5UIb64ss4QmtXY8SPIR9IDNvmnYUX4/Av/iKpZrSRHCHzpYX7Eecp2jdV/ta jgKpR4VaDyemyij093oW =LXas -END PGP SIGNATURE-
Re: wintrust/tests: make sure return values are used (LLVM/Clang) (resend)
On Fri, Feb 4, 2011 at 00:42, Paul Vriens wrote: > On 02/04/2011 09:32 AM, Austin English wrote: >> >> - ret = funcs->pfnObjectTrust(&data); >> + funcs->pfnObjectTrust(&data); >> + ok(ret == S_FALSE, "Expected S_FALSE, got %08x\n", ret); > > This doesn't look correct (you are removing the assignment and then check > it). I know that this code path will never be taken, but still. Fixed, thanks. Bedtime now, before I make any more typos :-). -- -Austin
Re: wintrust/tests: make sure return values are used (LLVM/Clang) (resend)
On 02/04/2011 09:32 AM, Austin English wrote: -ret = funcs->pfnObjectTrust(&data); +funcs->pfnObjectTrust(&data); +ok(ret == S_FALSE, "Expected S_FALSE, got %08x\n", ret); This doesn't look correct (you are removing the assignment and then check it). I know that this code path will never be taken, but still. -- Cheers, Paul.
Re: wintrust/tests: make sure return values are used (LLVM/Clang) (1/4)
On Thu, Feb 3, 2011 at 20:16, Juan Lang wrote: > Hey Austin, > > + ok(ret == S_FALSE, "Expected S_OK, got %08x\n", ret); > Copy/pasto. Thanks, resent. -- -Austin
Re: wintrust/tests: make sure return values are used (LLVM/Clang) (3/4)
On Thu, Feb 3, 2011 at 20:19, Juan Lang wrote: > Hey Austin, > > ok(ret, "WintrustSetRegPolicyFlags failed: %d\n", GetLastError()); > size = sizeof(flags1); > r = RegQueryValueExA(key, State, NULL, NULL, (LPBYTE)&flags1, &size); > - ok(flags1 == flags3, "Got %08x flags instead of %08x\n", flags1, flags3); > + ok(!r || r == ERROR_FILE_NOT_FOUND, "RegQueryValueEx failed: %d\n", r); > + if (!r) > + ok(flags1 == flags3, "Got %08x flags instead of %08x\n", > flags1, flags3); > > It seems to me that the introduction of ERROR_FILE_NOT_FOUND weakens > the test. (Testing r should be done, thanks for fixing that.) I > assume that, since WintrustSetRegPolicyFlags should have succeeded, > the registry value should now exist. Is there a test failure you're > also trying to fix? No, good catch. Too little caffeine -> copy/paste error. Resending, thanks. -- -Austin
