Re: [ros-dev] [ros-diffs] [greatlrd] 22718: Small clean up 1. Remove goto in the code, goto is slow and should be avoid. 2. reformat for adding {} around some code. 3. remove some NULL check after I did remove goto that is not longer need it.

[Saveliy Tretiakov]

I don't like your changes to main(), you added a lot of code 
dublication, made it harder to read and maintain. This is repeated three 
times:
            DeleteCriticalSection(&LogListCs);

// Close all log files.
            for(pLogf = LogListHead; pLogf; pLogf = ((PLOGFILE)pLogf)->Next)
            {
                LogfClose(pLogf);
            }

HeapDestroy(MyHeap);

[e-mail] wrote:
> Author: greatlrd
> Date: Fri Jun 30 20:42:21 2006
> New Revision: 22718
>
> URL: http://svn.reactos.org/svn/reactos?rev=22718&view=rev
> Log:
> Small clean up
> Remove goto in the code, goto is slow and should be avoid.
> reformat for adding {} around some code.
> remove some NULL check after I did remove goto that is not longer need it.
>
> Modified:
>     trunk/reactos/base/services/eventlog/eventlog.c
>
> Modified: trunk/reactos/base/services/eventlog/eventlog.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/eventlog/even...
> ==============================================================================
> --- trunk/reactos/base/services/eventlog/eventlog.c (original)
> +++ trunk/reactos/base/services/eventlog/eventlog.c Fri Jun 30 20:42:21 2006
> @@ -61,7 +61,7 @@
>    DWORD MaxValueLen, ValueLen, Type, ExpandedLen;
>    WCHAR *Buf = NULL, *Expanded = NULL;
>    LONG Result;
> BOOL ret = FALSE;
>
> BOOL ret = TRUE;
> PLOGFILE pLogf;
>
> DPRINT("LoadLogFile: %S\n", LogName);
>
> @@ -88,12 +88,14 @@
>    if(Result != ERROR_SUCCESS)
>    {
>    	DPRINT1("RegQueryValueEx failed: %d\n", GetLastError());
> goto cleanup;
> HeapFree(MyHeap, 0, Buf);
>    return FALSE;
> }
>  if(Type != REG_EXPAND_SZ && Type != REG_SZ)
>  {
>   DPRINT1("%S\File - value of wrong type %x.\n", LogName, Type);
> goto cleanup;
> HeapFree(MyHeap, 0, Buf);
>    return FALSE;
>
> }
>
> ExpandedLen = ExpandEnvironmentStrings(Buf, NULL, 0);
>
> @@ -102,7 +104,8 @@
>    if(!Expanded)
>    {
>    	DPRINT1("Can't allocate heap!\n");
> goto cleanup;
> HeapFree(MyHeap, 0, Buf);
>    return FALSE;
>
> }
>
> ExpandEnvironmentStrings(Buf, Expanded, ExpandedLen);
>
> @@ -114,14 +117,11 @@
>    if(pLogf == NULL)
>    {
>    	DPRINT1("Failed to create %S!\n", Expanded);
> goto cleanup;
> }
> ret = TRUE;
>
> -cleanup:
> ret = FALSE;
> }
> HeapFree(MyHeap, 0, Buf);
> if(Expanded) HeapFree(MyHeap, 0, Expanded);
> HeapFree(MyHeap, 0, Expanded);
> return ret;
>
> }
>
> @@ -129,8 +129,7 @@
>  {
>    LONG result;
>    DWORD MaxLognameLen, LognameLen;
> WCHAR *Buf = NULL;
> BOOL ret = FALSE;
>
> WCHAR *Buf = NULL;	
> INT i;
>
> RegQueryInfoKey(eventlogKey, NULL, NULL, NULL, NULL, &MaxLognameLen,
>
> @@ -143,7 +142,7 @@
>    if(!Buf)
>    {
>    	DPRINT1("Error: can't allocate heap!\n");
> return ret;
> return FALSE;
>
> }
>
> i = 0;
>
> @@ -159,7 +158,8 @@
>    	if(result != ERROR_SUCCESS) 
>    	{
>    		DPRINT1("Failed to open %S key.\n", Buf);
> 	goto cleanup;
> 	HeapFree(MyHeap, 0, Buf);
> 	return FALSE;
> }
>
> if(!LoadLogFile(SubKey, Buf))
>
> @@ -171,18 +171,14 @@
>    	i++;
>    }
> ret = TRUE;
>
> -cleanup:
>    HeapFree(MyHeap, 0, Buf);
> return ret;
> return TRUE;
>
> }
>
> INT main()
>  {
>    WCHAR LogPath[MAX_PATH];
> PLOGFILE pLogf;
> INT RetCode = 0;
> PLOGFILE pLogf;	
> LONG result;
> HKEY elogKey;
>
> @@ -193,8 +189,15 @@
>    if(MyHeap==NULL)
>    {
>    	DPRINT1("FATAL ERROR, can't create heap.\n");
> RetCode = 1;
> goto bye_bye;
> DeleteCriticalSection(&LogListCs);	
> // Close all log files.
> for(pLogf = LogListHead; pLogf; pLogf = ((PLOGFILE)pLogf)->Next)
> {
> 	LogfClose(pLogf);
> }
> return 1;
>
> }
>
> GetWindowsDirectory(LogPath, MAX_PATH);
>
> @@ -214,8 +217,17 @@
>    	if(result != ERROR_SUCCESS)
>    	{
>    		DPRINT1("Fatal error: can't open eventlog registry key.\n");
> 	RetCode = 1;
> 	goto bye_bye;
> 	DeleteCriticalSection(&LogListCs);
> 	// Close all log files.
> 	for(pLogf = LogListHead; pLogf; pLogf = ((PLOGFILE)pLogf)->Next)
> 	{
> 	    LogfClose(pLogf);
> 	}
> 	HeapDestroy(MyHeap);
> 	return 1;
> }
>
> LoadLogFiles(elogKey);
>
> @@ -223,16 +235,16 @@
>  StartServiceCtrlDispatcher(ServiceTable);
>
> -bye_bye:
>    DeleteCriticalSection(&LogListCs);
>
> // Close all log files.
>    for(pLogf = LogListHead; pLogf; pLogf = ((PLOGFILE)pLogf)->Next)
> {
> LogfClose(pLogf);
> }
> if(MyHeap) HeapDestroy(MyHeap);
> return RetCode;
> HeapDestroy(MyHeap);
> return 0;
>
> }
>
> VOID EventTimeToSystemTime(DWORD EventTime,