winetests: new addition

[Dimitrie O. Paun]

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.