[PATCH 3/3] kernel32/tests: Add a test for opening short paths of differing case.

Charles Davis cdavis at mymail.mines.edu
Wed Oct 27 22:47:52 CDT 2010


On 10/27/10 9:21 PM, Vitaliy Margolen wrote:
> On 10/27/2010 07:46 PM, Charles Davis wrote:
>> +    sprintf(buf,"%s\\%s\\%s",tmpdir,dirname,filename);
> Could you please put some spaces after each coma everywhere in your patch?
> Please use snprintf().
I was just following the surrounding style. But yeah, I guess I should
use snprintf() on a fixed-size buffer.
> 
>> +    GetShortPathNameA(buf,shortbuf,MAX_PATH);
> Use sizeof(buf) instead of MAX_PATH to guarantee that's what it means.
Good point.
> 
>> +    hndl =
>> CreateFileA(shortbuf,GENERIC_READ|GENERIC_WRITE,0,NULL,OPEN_EXISTING,0,NULL);
>>
>> +    ok(hndl!=INVALID_HANDLE_VALUE,"CreateFileA failed\n");
> It's nice to know LastError and the full filename if it fails.
The other tests don't do this. That'll teach me to go against my better
judgment.
> 
>> +    for(i=0;i<MAX_PATH&&  shortbuf[i];i++)
> You getting past the filename size in the buffer.
> 
>> +        shortbuf[i]=(rand() % 2) ? tolower(shortbuf[i]) :
>> toupper(shortbuf[i]);
> Please don't use any sort of random stuff in tests. They should always
> be the same every time on every platform.
I forgot about that rule. I'll change it to alternate upper and lowercase.
> 
>> +  /* Now try it on mixed case short names */
> What's the point of the test anyway? You know that Wine already handles
> all files as case-insensitive. Short file name isn't any different.
Don't blame me; this was AJ's idea. He told me, "you should probably
first write more tests with various case and short names combinations,"
when he was critiquing patch 2 in this series. I'm just "following orders."

Chip



More information about the wine-devel mailing list