Re[2]: findstr: Minimal implementation

Mikhail Bystryantsev ho-rr-or at mail.ru
Sun Aug 30 18:26:47 CDT 2015


> Hi, Mikhail.
> 
> Some comments:
> 
> > +static void __cdecl printError(unsigned int id, ...)
> 
> Why explicit __cdecl?
> 
> > +	WCHAR format[1024];
> 
> Here, and everywhere else - please use 4 spaces per indentation level 
> instead of a tab.
> 
> > +	if (!LoadStringW(GetModuleHandleW(NULL), id, format, sizeof(format) / sizeof(format[0])))
> 
> I think calling it as LoadStringW(NULL, ...) has same effect.
> 
> 
> > +int find(WCHAR* patternW, WCHAR* filename)
> 
> This should be static.
> 
> > +	WideCharToMultiByte(CP_OEMCP, 0, patternW, -1, pattern, patternLen, NULL, NULL);
> 
> Why OEMCP and not ACP?
> 
> > +					// Latest char, include them to line
> 
> Please avoid this C99/C++ comment style.
> 

Hi, Nikolay. Thanks for feedback.

> Why explicit __cdecl?
Ok, I will remove it. I don't know why, but saw it in other source file. I decided that maybe there is some reason.

>  Here, and everywhere else - please use 4 spaces per indentation level 
> instead of a tab. 
Ok. I met different indentations styles, so decided that it does not matter.

> I think calling it as LoadStringW(NULL, ...) has same effect.
Ok. I just tried to use WinApi as it used in other sources and do not go into details.

> This should be static.
> Please avoid this C99/C++ comment style.
Ok, my failures.

> Why OEMCP and not ACP?
My failure too. I will use GetConsoleCP() instead.


More information about the wine-devel mailing list