[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 at 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/eventlog.c?rev=22718&r1=22717&r2=22718&view=diff
> ==============================================================================
> --- 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, 
>
>
>
>