"Dimitrie O. Paun" <dpaun at 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.