On 05/27/15 04:35, YongHao Hu wrote:
> +/* filesystem */
> +static unsigned long long (__cdecl*p_tr2_sys__File_size)(char const*);
You should avoid using "long long" type. Please e.g. use ULONGLONG instead.
> + mkdir("tr2_test_dir");
> + file = CreateFileA("tr2_test_dir/f1", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL);
> + SetFilePointerEx(file, large, NULL, FILE_BEGIN);
> + SetEndOfFile(file);
Please test return values of CreateFile, SetFilePointerEx and SetEndOfFile.
> + large.LowPart = 4000000000u;
> + large.HighPart = 120;
> + file = CreateFileA("tr2_test_dir/f2", GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, NULL);
> + SetFilePointerEx(file, large, NULL, FILE_BEGIN);
> + SetEndOfFile(file);
> + val = p_tr2_sys__File_size("tr2_test_dir/f2");
> + ok(val == 519396075520 || broken(val == 0), "file_size is %llu\n", val); /* not enough storage on the testbots */
> + CloseHandle(file);
Please remove this test. It's not a good idea to create ~500GB file in
the tests.
> + val = p_tr2_sys__File_size("tr2_test_dir");
> + ok(val == 0, "file_size is %llu\n", val);
> +
> + val = p_tr2_sys__File_size("tr2_test_dir/not_exists_file");
> + ok(val == 0, "file_size is %llu\n", val);
It would be nice to test GetLastError and errno value after this calls.
What happens if NULL is passed as the argument?
> +ULONGLONG __cdecl tr2_sys__File_size(const char* path)
> +{
> + WIN32_FILE_ATTRIBUTE_DATA fad;
Please add a trace message.
> + if(!GetFileAttributesExA(path, GetFileExInfoStandard, &fad))
> + return (ULONGLONG)0;
This cast is not needed, you can just return 0 here.
> + if(fad.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
> + return (ULONGLONG)0;
> + return ((ULONGLONG)(fad.nFileSizeHigh) << (sizeof(fad.nFileSizeLow)*8)) + fad.nFileSizeLow;
I think that it's better to simply write 32 instead of
sizeof(fad.nFileSizeLow)*8 here.
Thanks,
Piotr