Re: programs/dxdiag: beginnings of support for /t option (try 2)

[Vitaliy Margolen]

On 03/12/2011 09:55 PM, Drew Goodwin wrote:
> Fixed issues pointed out by Andrew Nguyen and Henri Verbeet.

There are still number of things that needs fixing:
> +static BOOL PrepareFilename(WCHAR* filename, const enum OUTFORMAT format);
> +static void OutputPlainFile(FILE * hFile, DxDiag_Root g_dxdiagcom_root);
> +static void OutputSectionHeading(int section, FILE * hFile);
> +static void OutputNewline(FILE * hFile);
> +static void OutputWString(FILE * hFile, WCHAR * buffer);
> +static void OutputPlainPropertyTitle(int property, FILE * hFile);
> +static void OutputPlainProperty(int property, WCHAR * value, FILE * hFile);

Please rearrange your functions so you don't need forward declarations.
> - WideCharToMultiByte(CP_UTF8, 0, filename, -1, utf8Filename,
> -
>                    MAX_STRING_LEN, NULL, NULL);

Why utf8? Native creates plain text file which isn't a utf8. Haven't checked 
on different languages, but I think it would use some windows codepage 
instead of utf8.
> -
>    LoadStringW(NULL, STRING_ERROR_OPENING_FILE, messageText,
> -
>                MAX_STRING_LEN);

Please don't split file on 80-character boundary, all modern widescreen 
displays can comfortably display 100+ characters in huge font.
> - else if (format == XML)
> -
>    WINE_FIXME("/x unimplemented\n");

If it's not implemented, please remove all related code and add it back with 
xml implementation.
> - /* trailing periods are not allowed in windows filenames */
> - /* some characters not allowed in windows filenames */

You shouldn't need to perform these checks here. Kernel/ntdll already do all 
of that.
> +static void OutputWString(FILE * hFile, WCHAR * buffer)
> +{
> - utf8BufferSize = WideCharToMultiByte(CP_UTF8, 0, buffer, -1, NULL, 0, NULL, NULL);
> - utf8Buffer = HeapAlloc(GetProcessHeap(), 0, utf8BufferSize);

This has really big overhead. Please use stack buffer instead and only do 
allocation when the buffer it too small.
> +++ b/programs/dxdiag/cmdline.h
> +BOOL OutputToFile(WCHAR * filename, const enum OUTFORMAT format, DxDiag_Root g_dxdiagcom_root);

You don't need 2 header files for 3 external functions. Just forward declare 
them in the file that uses them. Or better yet put it all into the same file.

And last but not the least, you might want to fix dxdiagn.dll to return a 
bit more for system info then this:
-------------------
  System Information
-------------------
Time of this report:
        Machine name:
    Operating System:
            Language:
System Manufacturer:
        System Model:
                BIOS:
           Processor:
              Memory:
           Page File:
         Windows Dir: C:\windows
     DirectX Version: = "DirectX 9.0c (4.09.0000.0904)
DX Setup Parameters:

Vitaliy