[Wine-devel] Замечания по eterforcetest

[Vitaly Lipatov]

http://git.etersoft.ru/people/grosso/packages/timetests.git

Репозиторий называется неправильно. Надо опубликовать с оригинальным 
названием.

> diff --git a/Makefile b/Makefile
> index fc587a3..9fe5d2b 100644 (file)
> --- a/Makefile
> +++ b/Makefile
> @@ -4,11 +4,11 @@ PUBDIR=/var/ftp/pub/Etersoft/Eterforcetest
> 
>  CFLAGS+=-Wall -O0 -g -Werror
>  LDFLAGS=
> 
> -LIBADD=-lgdi32 -lole32 $(MINGWLIBDIR)/libuuid.a
> +LIBADD=-lgdi32 -lole32 $(MINGWLIBDIR)
Зачем удалил?

Надо читать коммиты, что в них нет лишнего.

> +#include <conio.h>
Вряд ли нужно

> +/*
> + *Search file function
Бессмысленный комментарий. Нужно писать, не что делает, а как делает.

> + */
> +BOOL SearchFiles(LPCTSTR lpszFileName, LPSEARCHFUNC lpSearchFunc, BOOL
> bInnerFolders) +{
> +    LPTSTR part;
> +    char tmp[MAX_PATH];
> +    char name[MAX_PATH];
> +
> +    HANDLE hSearch = NULL;
> +    WIN32_FIND_DATA wfd;
> +    memset(&wfd, 0, sizeof(WIN32_FIND_DATA));
> +
> +    if(bInnerFolders)
> +    {
> +        if(GetFullPathName(lpszFileName, MAX_PATH, tmp, &part) == 0)
> return FALSE; 
> +        strcpy(name, part);
> +        strcpy(part, "*.*");
Какое-то сомнительное копирование. А ты сам функцию писал или откуда-то?

> +/*
> + *Change reg
> + */
> +void get_big(char *a)
> +{
> +               int len_a = 25;
> +               int len_b = 10;
> +               int i;
> +               for(i = len_b; i<len_a -6; i++)
> +               {
> +                               if((int)a[i]<=95)
> +                                       a[i] = (int)a[i]+32;
> +                               else
> +                                       a[i] = (int)a[i]-32;
> +               }
> +}
Не понимаю, что делает эта функция. Думаю, она не нужна.


> +/*
> + *Generate File Name (use 4 formats)
> + *Generate search mask
> + */
> +void GenerateFileName(char *a, char *b, char *c)
> +{
> +       char format1[5] = {'.', 't', 'x', 't', 0};
> +       char format2[5] = {'.', 'm', 'k', 'w', 0};
> +       char format3[5] = {'.', 'c', 'p', 'p', 0};
> +       char format4[5] = {'.', 'd', 'o', 'k', 0};
Разные расширения — лишнее.

> +       char mask[3]={'.', '*', 0};
Не надо использовать W-функции, и не придётся так строки записывать.

> +       int len_a = 25;
> +       int len_b = 10;
> +       int len_c = 12;
не должно быть констант.

> +       int buf = 0;
buf не может иметь тип int

> +       int i;
> +       int form  = 0;
> +       strcpy( a, b);
> +       a[len_b-1]='/';
> +       buf = rand() % 2 + 0;
> +       if( buf == 0)
> +       {
> +       for ( i = len_b; i<len_a-6; i++)
> +               {
> +                       a[i] = rand() % 26 + 65;   //mean "A" - "Z"
Тут нужно было написать хотя бы rand()%('Z'-'A') + 'A'

> +                       c[i-(len_b)] = a[i];
> +               }
> +       }
> +       else
> +       {
> +               for ( i = len_b; i<len_a-6; i++)
> +               {
> +                       a[i] = rand() % 26 + 97;   //mean "a" - "z"
> +                       c[i-(len_b)] = a[i];
> +               }
> +       }
Второй раз ту же конструкцию писать не стоит. Допустим, константы надо просто 
задать снаружи цикла и убрать условие с buf.


> +void test_stat()
> +{
> +       printf("\n\n\n\n Runing stat test  ");
> +       FILE *hFile;           //for create files
> +       int hFile1 = 0;        //for get file informatioun]
Переменные, которые используются только внутри какого-то блока, стоит там и 
объявлять, сокращая им область видимости.

> +       char fn[25];           //file which we wont known
> +       SYSTEMTIME st;
> +       struct stat BufInfo;   //contain file information
> +       char name_dir[10] = { 'c', 'o', 'n', 't', 'a', 'i', 'n', 'e', 'r',
Опять же. не надо W-функций и таких строк.

> 0};     //name dir +       char mas[25];             //contain files name
> +       char search[12];          //contain masck (need for search)
> +       int i;
> +       char mass_name_of_files[1000][25];   //use for delet files
Для размера буфера пути к файлу есть константа

> +       int dum = 0;
> +       srand ( time(NULL) );
> +       GetLocalTime(&st);
Зачем?

> +       /*
> +       *Create new directroy which contain files
> +       */
> +       if(CreateDirectory(name_dir,NULL))
Странно, как это работало...

> +       {
> +               printf("\nDirectory was create...");
> +       }
> +       for(i=0;i<1000;i++)
Думаю, что 1000 файлов — мало. Лучше 10000 тыс.

> +       {
> +               GenerateFileName(mas, name_dir, search);
> +               strcpy(mass_name_of_files[i],mas);
> +               hFile = CreateFile(mas, GENERIC_WRITE, FILE_SHARE_READ,
> NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); +              
> CloseHandle(hFile);
> +               dum++;
> +       }
> +       printf("\n%d Files was create...", dum);
Вряд ли нужно считать dum и выводить

> +       printf("\nYou searching  %s ", search);
> +       SearchFiles(search, DoSomething, TRUE);
Боюсь, что функция не понадобится.

> +       printf("\n\tfstat() for \t\n  %s ", mas);
Лишний вывод

> +       hFile1 = open(mas, O_RDONLY);
И что тут в mas?

> +       MSTART(1, "fstat", 20)
> +       {
> +               fstat(hFile1, &BufInfo);
> +       }MEND
Давай для однотипности тут использовать CreateFile.
Вообще суть была в том, чтобы сравнить время открытия файла.
Так что внутрь MSTART стоит разместить CreateFile и всё.
И, конечно, количество циклов не должно быть 20. Видимо, какие-то десятки 
тысяч. Так, чтобы измерение проходило секунд 10.

> +       close(hFile1);
> +       hFile1 = open(fn, O_RDONLY);
> +       get_big(mas);
Что тут изменилось?
Тест должен позволять сравнить скорость поиска файла при совпадении регистра с 
реальным (на файловом системе) и при несовпадении.
Видимо, поиск надо выполнять для всех созданных файлов.

> +       printf("\n\tfstat() for \t\n  %s ", mas);
> +       hFile1 = open(mas, O_RDONLY);
> +       MSTART(1, "fstat", 20)
> +       {
> +               fstat(hFile1, &BufInfo);
> +       }MEND
> +       close(hFile1);
> +       for(i=0;i<1000;i++)
Константу задать через define

> +       {
> +               DeleteFile(mass_name_of_files[i]);
> +       }
> +       RemoveDirectory(name_dir);
> +       printf("\n Remove all files and directory was complite.\n");
complete
Вообще сообщения стоит выводить только в случае неудачи.
То есть надо проверять результат выполнения функции.

> +       printf("\n ...Test was completed... \n");
> +}
-- 
С уважением,
Виталий Липатов
Россия, Санкт-Петербург. www.etersoft.ru
GNU! ALT Linux Team! WINE! WIKI! LaTeX! LyX!