"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
I'm still not sure how to support building with PE .exes
Could you outline the problems you can see? Since it is still not in the tree, instead of patches I send comments and questions.
+#include <shellapi.h> +#include <shellapi.h>
Do we really need it twice?
+ while (1) + { + va_start(ap, fmt); + n = vsnprintf (p, size, fmt, ap); + va_end(ap); + if (n > -1 && n < size) return p; + size = min( size*2, n+1 ); + if(!(p = realloc (p, size))) fatal("Out of memory."); + }
MSDN has no vnsprintf() only _vsnprintf(), what is this? The size=min(...) does not make sense for me either. va_start and va_end could be moved out of the loop by using break instead of return.
+int print_version(FILE *fout) +{ + OSVERSIONINFOEX ver; + + ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX); + if (!GetVersionEx ((OSVERSIONINFO *) &ver)) + { + ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + if (!GetVersionEx ((OSVERSIONINFO *) &ver)) + return 0;
This should be made a fatal(), I think.
+ } + fprintf(fout, " dwMajorVersion=%ld\n dwMajorVersion=%ld\n"
The second is dwMinorVersion.
+void deletePath( char *path ) +{ + SHFILEOPSTRUCT fileop; + + path[strlen( path ) + 1] = '\0';
We do not have space allocated for this NUL. Here we use the advanced Shell API but later not the Win32 API for opening files and such. What is our policy?
+int count_tests()
This is superfluous, not used for anything particularly useful. The progress bar still did not make it...
+ if((fout = fopen(test->exename, "wb"))) + { + fwrite(code, size, 1, fout); + fclose(fout); + }
No error checking whatsoever...
+ char line[512], *cmd;
Fixed size buffers are wrong...
+ if ((fp = popen( cmd, "r" ))) + { + while (fgets( line, sizeof(line), fp )) + fprintf( logfp, "%s", line ); + fclose( fp ); + }
Why not a CreateProcess with redirected stderr/stdout?
+ if (!(fp = fopen( logfile, "w" ))) fatal("Could not open log file.");
Why not append mode? That would also make the above redirection more secure.
+ for(test = wine_tests; test->name; test++)
The number of tests could be determined by sizeof instead of a special member, but I am not sure that would be worth it.
+BINDIR="../.." [...] +for test in $TEST_EXES; do + testname=`basename $test .exe` + filename="$BINDIR/$test.so"
I am probably too tired, but can not see how you find the test .so-s in the dlls directory... So I am back, and ready to go for finishing this project... Feri.
On October 18, 2003 07:04 am, Ferenc Wagner wrote:
+#include <shellapi.h> +#include <shellapi.h>
Do we really need it twice?
Of course not :)
+ while (1) + { + va_start(ap, fmt); + n = vsnprintf (p, size, fmt, ap); + va_end(ap); + if (n > -1 && n < size) return p; + size = min( size*2, n+1 ); + if(!(p = realloc (p, size))) fatal("Out of memory."); + }
MSDN has no vnsprintf() only _vsnprintf(), what is this?
It should have a vnsprintf() into some import lib oldnames IIRC.
The size=min(...) does not make sense for me either.
Hmmm, I can't remember why the min either. It should be more like so: size = (n < 0) ? size*2 : n + 1;
va_start and va_end could be moved out of the loop by using break instead of return.
This was just copied from tools/winegcc/utils.c. Feel free to improve it, but please fix it in both places.
+int print_version(FILE *fout) +{ + OSVERSIONINFOEX ver; + + ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX); + if (!GetVersionEx ((OSVERSIONINFO *) &ver)) + { + ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + if (!GetVersionEx ((OSVERSIONINFO *) &ver)) + return 0;
This should be made a fatal(), I think.
Indeed.
+ } + fprintf(fout, " dwMajorVersion=%ld\n dwMajorVersion=%ld\n"
The second is dwMinorVersion.
Yes, copy&paste error.
+void deletePath( char *path ) +{ + SHFILEOPSTRUCT fileop; + + path[strlen( path ) + 1] = '\0';
We do not have space allocated for this NUL.
Here we use the advanced Shell API but later not the Win32 API for opening files and such. What is our policy?
No policy. Jakob wrote this one, and I just used it. If it were the only thing imported from shell32, I'd get rid of it, but we also need the ShellExecute().
+int count_tests()
This is superfluous, not used for anything particularly useful. The progress bar still did not make it...
Right, but I hope we'll have one eventually.
+ if((fout = fopen(test->exename, "wb"))) + { + fwrite(code, size, 1, fout); + fclose(fout); + }
No error checking whatsoever...
Yeah, I was lazy, sue me :P
+ char line[512], *cmd;
Fixed size buffers are wrong...
Oh come on, it's just a temp buffer to copy stuff through. It introduces no limitation.
+ if ((fp = popen( cmd, "r" ))) + { + while (fgets( line, sizeof(line), fp )) + fprintf( logfp, "%s", line ); + fclose( fp ); + }
Why not a CreateProcess with redirected stderr/stdout?
Sure.
+ if (!(fp = fopen( logfile, "w" ))) fatal("Could not open log file.");
Why not append mode? That would also make the above redirection more secure.
Why append mode? How is it making it more secure?
+ for(test = wine_tests; test->name; test++)
The number of tests could be determined by sizeof instead of a special member, but I am not sure that would be worth it.
I know, it was simple/cleaner to write the loop like so.
+BINDIR="../.."
[...]
+for test in $TEST_EXES; do + testname=`basename $test .exe` + filename="$BINDIR/$test.so"
I am probably too tired, but can not see how you find the test .so-s in the dlls directory...
I don't find anything. TEST_EXES is a list of tests, stuff that looks like this: dlls/user/tests/user32.exe dlls/gdi/tests/gdi32.exe ... etc ... BINDIR is points to the root of your build dir, something like /home/dimi/dev/wine/wine.build This bit works, did I miss anything?
So I am back, and ready to go for finishing this project...
Cool, feel free to change the program as you wish. -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
On October 18, 2003 07:04 am, Ferenc Wagner wrote:
+ char line[512], *cmd;
Fixed size buffers are wrong...
Oh come on, it's just a temp buffer to copy stuff through. It introduces no limitation.
Sorry, I was too quick here.
+ while (fgets( line, sizeof(line), fp )) + fprintf( logfp, "%s", line );
But here is the reason: I was sure fgets was deprecated. For whole bunch of libc input functions this is for possible buffer overruns, but not for fgets. The problem here is that you can not tell a NUL in the input stream.
+ if (!(fp = fopen( logfile, "w" ))) fatal("Could not open log file.");
Why not append mode? That would also make the above redirection more secure.
Why append mode? How is it making it more secure?
In principle, if something went wrong in a test, it could seek back in its output stream. We are only appending anyway.
This bit [BINDIR] works, did I miss anything?
No, I was confused. No wonder, I am not at all familiar with the build system... Have you got an idea why your patch has not been committed? Feri.
On October 19, 2003 01:05 pm, Ferenc Wagner wrote:
But here is the reason: I was sure fgets was deprecated. For whole bunch of libc input functions this is for possible buffer overruns, but not for fgets. The problem here is that you can not tell a NUL in the input stream.
Yes, I know, I don't think it's terribly important here, feel free to change it if you wish. However, I think we'd better stick to the standard C functions for file IO, we might not want to make this program dependent on wine on Linux (why, I don't know :)).
Have you got an idea why your patch has not been committed?
Not really. Maybe because I haven't solved the cross compilation problem... Alexandre was rather busy with the DLL separation work, I didn't want to bother him with this stuff. But now that he finished... <g> -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
On October 19, 2003 01:05 pm, Ferenc Wagner wrote:
Have you got an idea why your patch has not been committed?
Not really. Maybe because I haven't solved the cross compilation problem... Alexandre was rather busy with the DLL separation work, I didn't want to bother him with this stuff. But now that he finished... <g>
It is a shame, but I can not apply your patch... It gives: $ patch -p0 <~winetest.patch patching file configure.ac Hunk #1 succeeded at 1580 (offset 3 lines). patching file programs/winetests/Makefile.in patch: **** malformed patch at line 48: --- /dev/null 7 Oct 2003 05:24:37 -0500 Line 48 is the second --- /dev/null line. As you can see, I tried to tweak the date format, insert empty lines etc., but nothing helped. I really feel stupid... Feri.
"Dimitrie O. Paun" <dimi(a)intelliware.ca> writes:
here is a tar.gz of the new files.
Thanks, that is fine. I came across a webpage http://www.websteves.com/win32asm/datrsrc.htm stating that DISCARDABLE, LOADONCALL etc. are not needed anymore. Do we still need them? Feri.
On October 21, 2003 07:43 pm, Ferenc Wagner wrote:
Thanks, that is fine. I came across a webpage http://www.websteves.com/win32asm/datrsrc.htm stating that DISCARDABLE, LOADONCALL etc. are not needed anymore. Do we still need them?
Good point, I'll get rid of them. -- Dimi.
Dimitrie O. Paun wrote:
+void deletePath( char *path ) +{ + SHFILEOPSTRUCT fileop; + + path[strlen( path ) + 1] = '\0';
We do not have space allocated for this NUL.
Se later down this message...
Here we use the advanced Shell API but later not the Win32 API for opening files and such. What is our policy?
No policy. Jakob wrote this one, and I just used it. If it were the only thing imported from shell32, I'd get rid of it, but we also need the ShellExecute().
My thought exactly. I also tried to write a recursive delete using plain Win32 but failed miserably. :-)
+ char line[512], *cmd;
Fixed size buffers are wrong...
Oh come on, it's just a temp buffer to copy stuff through. It introduces no limitation.
Yep. Same thing with MAXPATH. I think I set some buffers to MAXPATH + 2 or something. Hence the extra null is accounted for. (Why is the code not in CVS BTW? I don't know where to look.)
Why not append mode? That would also make the above redirection more secure.
Why append mode? How is it making it more secure?
I tried append mode. That screwed the file up totally when tests crashed on Win98. So I changed to "w". I don't know if this applies at all to how Dimi rewrote the program. regards, Jakob
Hi there, below is my present vision of main.c. It does not work: seems like the spawnvp call does nothing. Can you give me a clue? Feri. /* * Wine Conformance Test EXE * * Copyright 2003 Jakob Eriksson (for Solid Form Sweden AB) * Copyright 2003 Dimitrie O. Paun * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. * * This library is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software * * This program is dedicated to Anna Lindh, * Swedish Minister of Foreign Affairs. * Anna was murdered September 11, 2003. * */ #include "config.h" #include "wine/port.h" #include <stdio.h> #include <stdlib.h> #include <errno.h> #include <ctype.h> #include <string.h> #include <unistd.h> #include <sys/stat.h> #include <wtypes.h> #include <shellapi.h> #include "winetests.h" void fatal(const char* msg) /* Funny when out of memory... */ { MessageBox( NULL, msg, "Fatal Error", MB_ICONERROR | MB_OK ); exit(1); } void *xmalloc (size_t len) { void *p; p = malloc (len); if (!p) fatal ("Out of memory."); return p; } void xprintf (const char *fmt, ...) { va_list ap; va_start (ap, fmt); if (vprintf (fmt, ap) < 0) fatal ("Can't write logs."); va_end (ap); } char *strmake(const char *fmt, ...) { int n; size_t size = 100; char *p = xmalloc (size); va_list ap; va_start(ap, fmt); while (1) { n = vsnprintf (p, size, fmt, ap); if (n > -1 && n < size) break; size = (n < 0) ? size*2 : n + 1; if(!(p = realloc (p, size))) fatal("Out of memory."); } va_end(ap); return p; } void print_version () { OSVERSIONINFOEX ver; BOOL ext; ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFOEX); if (!(ext = GetVersionEx ((OSVERSIONINFO *) &ver))) { ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); if (!GetVersionEx ((OSVERSIONINFO *) &ver)) fatal("Can't get OS version."); } xprintf (" dwMajorVersion=%ld\n dwMinorVersion=%ld\n" " dwBuildNumber=%ld\n PlatformId=%ld\n szCSDVersion=%s\n", ver.dwMajorVersion, ver.dwMinorVersion, ver.dwBuildNumber, ver.dwPlatformId, ver.szCSDVersion); if (ext) xprintf (" wServicePackMajor=%d\n wServicePackMinor=%d\n" " wSuiteMask=%d\n wProductType=%d\n wReserved=%d\n", ver.wServicePackMajor, ver.wServicePackMinor, ver.wSuiteMask, ver.wProductType, ver.wReserved); } void deletePath( const char *path ) { SHFILEOPSTRUCT fileop; size_t len = strlen (path); char *paths = xmalloc (len+1); memcpy (paths, path, len); paths[len] = 0; fileop.hwnd = NULL; fileop.wFunc = FO_DELETE; fileop.pFrom = paths; fileop.pTo = NULL; fileop.fFlags = FOF_NOCONFIRMATION | FOF_NOERRORUI; if (SHFileOperation( &fileop )) /* FIXME not present in NT3 */ fatal (strmake ("Can't delete temporary directory: %s.", path)); } int count_tests() { struct wine_test* test; int total = 0; for (test = wine_tests; test->name; test++) total += test->subtest_count; return total; } LPVOID extract_rcdata (const char *name, DWORD* size) { HRSRC rsrc; HGLOBAL hdl; if (!(rsrc = FindResource( 0, name, RT_RCDATA ))) return 0; if (!(*size = SizeofResource( 0, rsrc ))) return 0; if (!(hdl = LoadResource( 0, rsrc ))) return 0; return LockResource(hdl); } void extract_test (const char *dir, struct wine_test* test) { BYTE* code; DWORD size; FILE* fout; if (!(code = extract_rcdata(test->name, &size))) fatal (strmake ("Can't get resource %s.", test->name)); test->is_elf = (code[1] == 'E' && code[2] == 'L' && code[3] == 'F'); test->exename = strmake("%s/%s.exe%s", dir, test->name, test->is_elf ? ".so" : ""); if (!(fout = fopen(test->exename, "wb")) || (fwrite (code, size, 1, fout) != 1) || fclose (fout)) fatal (strmake("Failed to write file %s.", test->name)); } void run_test (struct wine_test* test, const char* subtest) { int status; const char *argv[]={test->exename, subtest, NULL}; xprintf ("%s:%s start\n", test->name, subtest); status = spawnvp (_P_WAIT, test->exename, argv); if (status) xprintf ("Exit status: %d, errno=%d: %m\n", status, errno); xprintf ("%s:%s done\n", test->name, subtest); } int WINAPI WinMain( HINSTANCE hInst, HINSTANCE hPrevInst, LPSTR cmdline, int cmdshow ) { struct wine_test* test; int nr_of_tests, subtest; char *tempdir, *logfile; FILE *fp; SetErrorMode( SEM_FAILCRITICALERRORS ); if (!(tempdir = tempnam(0, "wct"))) fatal("Can't name temporary dir (check TMP)."); fprintf (stderr, "tempdir=%s\n", tempdir); if (mkdir( tempdir, 0777 )) fatal(strmake("Could not create directory: %s", tempdir)); if (chdir( tempdir )) fatal(strmake("Could not make %s current directory.", tempdir)); if (!(logfile = tempnam(0, "res"))) fatal("Can't name log file."); fprintf (stderr, "logfile=%s\n", logfile); if (!(fp = fopen( logfile, "a" ))) fatal("Could not open log file."); if (-1 == dup2 (fileno (fp), 1)) fatal ("Can't redirect stdout."); xprintf ("Tests from build %s\n", "FIXME" ); xprintf ("Operating system version:\n"); print_version (); xprintf ("Test output:\n" ); for (test = wine_tests; test->name; test++) extract_test (tempdir, test); nr_of_tests = count_tests(); for (test = wine_tests; test->name; test++) for (subtest = 0; subtest < test->subtest_count; subtest++) run_test (test, test->subtests[subtest]); fflush (stdout); fclose (fp); deletePath( tempdir ); return 0; }
Ferenc Wagner wrote:
Here we use the advanced Shell API but later not the Win32 API for opening files and such. What is our policy?
Aha... now I understand what you mean. The fopen instead of some Win32-ism. Also inherited from my version of the program, no doubt. I just used cause I was familiar with it. I guess it would be looking to use some Win32 - stuff.
participants (4)
-
Dimitrie O. Paun -
Dimitrie O. Paun -
Ferenc Wagner -
Jakob Eriksson