[Wine-devel] [Wine-patches] [eterhack] Reimplement Etersoft Authors and output Etersoft informations (eterbug #6494). Замечания

[Vitaly Lipatov]

On 15 января 2011, grosso wrote:
> --- a/dlls/shell32/Makefile.in
> +++ b/dlls/shell32/Makefile.in
> @@ -1,4 +1,4 @@
> -EXTRADEFS = -D_SHELL32_ -DCOM_NO_WINDOWS_H
> +EXTRADEFS = -D_SHELL32_ -DCOM_NO_WINDOWS_H
> -D_FORTIFY_SOURCE=1


Не вижу смысла добавлять -D_FORTIFY_SOURCE=1
 
> --- /dev/null
> +++ b/dlls/shell32/etersoft_about.c
> @@ -0,0 +1,294 @@
> +#define ETERSOFT_FUNC_LICENSE
> +#define ETERSOFT_FUNC_VERSION

Эти дефайны не нужны.

> +void add_etersoft_authors(HINSTANCE hInstance, HWND list ,
> const WCHAR *authors ) +{
Тут лучше добавить комментарий, что функция полностью скопирована из такой-то.
И уже нет смысла передавать сюда authors, достаточно определить внутри функции,
как в оригинале.
И почему функция перестала быть static, как в оригинале?

> +void fill_listbox(HWND hWndCtl, const char* const *pstr,
> HFONT hFont) +{
Зачем нужна эта функция и где она используется?

> ); +	/*
> +	* add etersoft_authors
> +	*/
> +	hWndCtl = GetDlgItem(hWnd, IDC_ABOUT_LISTETER);
> +	ShowWindow( hWndCtl, SW_HIDE );
> +	add_etersoft_authors( shell32_hInstance ,hWndCtl ,
> authors_etersoft ); +
ShowWindow и пр. функции вокруг при добавлении авторов стоит вызывать также, как
и вокруг оригинальной функции.

> +
> +	while ((dwBytesRead = read(hFile,buf,1024))!=0)
> +        dwFileSize += dwBytesRead;
> +	hMem = GlobalAlloc(GHND,dwFileSize);
Просьба переделать вычисление размера файла путём его считывания.

> +	if (hMem!=NULL)
Условие лучше инвертировать — выйти сразу, если hMem == NULL

> +}
> +
> +
> +void etersoft_paint_picture(RECT Rec, HDC hDC, HWND hWnd, int
> width, int height) +{
Нет смысла передавать сюда обнулённые width, height

> +	if (pPicture == NULL)
> +		return;
> +
> +	IPicture_get_Width(pPicture,&width);
> +    IPicture_get_Height(pPicture,&height);
Они заполняются здесь

> --- a/dlls/shell32/shell32_main.c
> +++ b/dlls/shell32/shell32_main.c
> @@ -48,6 +48,8 @@
> 
>  #include "wine/debug.h"
>  #include "wine/unicode.h"
> 
> +#include "wine/etersoft.h"
Зачем нужен этот include?

> +#include "etersoft_about.h"
> 
>  WINE_DEFAULT_DEBUG_CHANNEL(shell);
> 
> @@ -938,7 +940,7 @@ static void add_authors( HWND list )
> 
>      HRSRC rsrc = FindResourceW( shell32_hInstance, authors,
>      (LPCWSTR)RT_RCDATA ); char *strA = LockResource(
>      LoadResource( shell32_hInstance, rsrc )); DWORD sizeW,
>      sizeA = SizeofResource( shell32_hInstance, rsrc );
> 
> -
> +
Не должно быть изменений

> @@ -964,8 +966,10 @@ static void add_authors( HWND list )
> 
>  static INT_PTR CALLBACK AboutDlgProc( HWND hWnd, UINT msg,
>  WPARAM wParam,
>  
>                                        LPARAM lParam )
>  
>  {
> 
> +
> 
>      HWND hWndCtl;
> 
> +
> 
>      TRACE("\n");
Лишние вставленные строки, надо убрать
>              WCHAR template[512], buffer[512], version[64];
>              extern const char *wine_get_build_id(void);
> 
> -
> +            #endif
> +
> 
Не должно быть второго +

>                  wine_get_build_id(), -1,
> 
> -                                     version,
> sizeof(version)/sizeof(WCHAR) ); 
> +                           
>             version, sizeof(version)/sizeof(WCHAR) );
Ненужное изменение

 
>                  hWndCtl = GetDlgItem(hWnd,

>                  IDC_ABOUT_LISTBOX);
> 
> -                SendMessageW( hWndCtl, WM_SETREDRAW, 0, 0 );
> -                SendMessageW( hWndCtl, WM_SETFONT,
> (WPARAM)info->hFont, 0 );
> 
>                  add_authors( hWndCtl );
> 
> -                SendMessageW( hWndCtl, WM_SETREDRAW, 1, 0 );
> -            }
> +                ShowWindow( hWndCtl, SW_SHOW );
> +           }
Не вижу, зачем здесь удалять SendMessage и добавлять ShowWindow.
 
>          }
>          return 1;
> 
> 
>      case WM_PAINT:
> -        {
> +        {
Здесь не должно быть изменения
 
>              PAINTSTRUCT ps;
> 
> +            RECT Rec;
> 
>              HDC hDC = BeginPaint( hWnd, &ps );
>              paint_dropline( hDC, hWnd );
> 
> +            etersoft_paint_picture(Rec, hDC, hWnd, 0, 0);
Странно передавать неинициализованный Rec. Передавать нули тоже не надо.

-- 
С уважением,
Виталий Липатов, ALT Linux Team, Eternity Software Team
Россия, Санкт-Петербург. http://etersoft.ru
GNU! ALT Linux! WINE! LaTeX! LyX! http://freesource.info