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);

greatlrd(a)svn.reactos.org 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
>  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.  
>   
>
>  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/eve…
>  ==============================================================================
>  --- 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, 
>
>
>
>