[jmorlan] 38910: cmd_move: - Only check for options at the beginning of the command line (subsequent code already assumed the filenames were at the end). - Give an error message if too many parameters are given. - If no destination is given, default to current directory. - Replace excessively complicated code to get source directory with single GetFullPathName call; hopefully that is sufficient. Check for pszFile == NULL to prevent crash that occurred when source was "/..". - To determine whethe

Author: jmorlan
Date: Sun Jan 18 14:21:03 2009
New Revision: 38910

URL: http://svn.reactos.org/svn/reactos?rev=38910&view=rev
Log:
cmd_move:
- Only check for options at the beginning of the command line (subsequent code already
assumed the filenames were at the end).
- Give an error message if too many parameters are given.
- If no destination is given, default to current directory.
- Replace excessively complicated code to get source directory with single GetFullPathName
call; hopefully that is sufficient. Check for pszFile == NULL to prevent crash that
occurred when source was "/..".
- To determine whether source wildcard matches are files or directories, just use
WIN32_FIND_DATA's dwFileAttributes; it's easier than constructing paths to pass to
IsExistingFile/IsExistingDirectory.
- Fix memory leaks: some returns were missing freep(arg).

Modified:
    trunk/reactos/base/shell/cmd/move.c

Modified: trunk/reactos/base/shell/cmd/move.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/shell/cmd/move.c?rev=…
==============================================================================
--- trunk/reactos/base/shell/cmd/move.c [iso-8859-1] (original)
+++ trunk/reactos/base/shell/cmd/move.c [iso-8859-1] Sun Jan 18 14:21:03 2009
@@ -87,6 +87,7 @@
 {
 	LPTSTR *arg;
 	INT argc, i, nFiles;
+	LPTSTR pszDest;
 	TCHAR szDestPath[MAX_PATH];
 	TCHAR szFullDestPath[MAX_PATH];
 	TCHAR szSrcDirPath[MAX_PATH];
@@ -138,73 +139,68 @@
 
 	nErrorLevel = 0;
 	arg = splitspace(param, &argc);
-	nFiles = argc;
 
 	/* read options */
 	for (i = 0; i < argc; i++)
 	{
-		if (*arg[i] == _T('/'))
-		{
-			if (_tcslen(arg[i]) >= 2)
-			{
-				switch (_totupper(arg[i][1]))
-				{
-				case _T('N'):
-					dwFlags |= MOVE_NOTHING;
-					break;
-				
-				case _T('Y'):
-					dwFlags |= MOVE_OVER_YES;
-					break;
-				
-				case _T('-'):
-					dwFlags |= MOVE_OVER_NO;
-					break;
-				}
-			}
-			nFiles--;
-		}
-	}
-	
-	if (nFiles < 2)
-	{
-		/* there must be at least two pathspecs */
-		error_req_param_missing ();
-		return 1;
-	}
-	
+		if (!_tcsicmp(arg[i], _T("/N")))
+			dwFlags |= MOVE_NOTHING;
+		else if (!_tcsicmp(arg[i], _T("/Y")))
+			dwFlags |= MOVE_OVER_YES;
+		else if (!_tcsicmp(arg[i], _T("/-Y")))
+			dwFlags |= MOVE_OVER_NO;
+		else
+			break;
+	}
+	nFiles = argc - i;
+
+	if (nFiles < 1)
+	{
+		/* there must be at least one pathspec */
+		error_req_param_missing();
+		freep(arg);
+		return 1;
+	}
+
+	if (nFiles > 2)
+	{
+		/* there are more than two pathspecs */
+		error_too_many_parameters(param);
+		freep(arg);
+		return 1;
+	}
+
+	/* If no destination is given, default to current directory */
+	pszDest = (nFiles == 1) ? _T(".") : arg[i + 1];
+
 	/* check for wildcards in source and destination */
-	if (_tcschr (arg[argc - 1], _T('*')) != NULL || _tcschr (arg[argc - 1],
_T('?')) != NULL)
+	if (_tcschr(pszDest, _T('*')) != NULL || _tcschr(pszDest, _T('?')) !=
NULL)
 	{
 		/* '*'/'?' in dest, this doesnt happen.  give folder name instead*/
-		error_invalid_parameter_format(arg[argc - 1]);
-		return 1;
-	}
-	if (_tcschr (arg[argc - 2], _T('*')) != NULL || _tcschr (arg[argc - 2],
_T('?')) != NULL)
+		error_invalid_parameter_format(pszDest);
+		freep(arg);
+		return 1;
+	}
+	if (_tcschr(arg[i], _T('*')) != NULL || _tcschr(arg[i], _T('?')) !=
NULL)
 	{
 		dwMoveStatusFlags |= MOVE_SOURCE_HAS_WILD;
 	}
 	
 	
 	/* get destination */
-	GetFullPathName (arg[argc - 1], MAX_PATH, szDestPath, NULL);
+	GetFullPathName (pszDest, MAX_PATH, szDestPath, NULL);
 	TRACE ("Destination: %s\n", debugstr_aw(szDestPath));
 	
 	/* get source folder */
-	GetDirectory(arg[argc - 2], szSrcDirPath, 1);
-	GetFullPathName(szSrcDirPath, MAX_PATH, szSrcPath, &pszFile);
-	_tcscpy(szSrcDirPath,szSrcPath);
-	/* we need following check to see if source happens to be directly given directory
-	and if it is then rip off last directory part so that there won't be any clashes
with codes after this point */
-	GetFullPathName(arg[argc - 2], MAX_PATH, szSrcPath, &pszFile);
-	if (_tcscmp(szSrcDirPath,szSrcPath) == 0)
-		szSrcDirPath[pszFile - szSrcPath] = _T('\0');
+	GetFullPathName(arg[i], MAX_PATH, szSrcDirPath, &pszFile);
+	if (pszFile != NULL)
+		*pszFile = _T('\0');
 	TRACE ("Source Folder: %s\n", debugstr_aw(szSrcDirPath));
 	
-	hFile = FindFirstFile (arg[argc - 2], &findBuffer);
+	hFile = FindFirstFile (arg[i], &findBuffer);
 	if (hFile == INVALID_HANDLE_VALUE)
 	{
-		ErrorMessage (GetLastError (), arg[argc - 2]);
+		ErrorMessage (GetLastError (), arg[i]);
 		freep (arg);
 		return 1;
 		
@@ -227,22 +223,14 @@
 	}
 	
 	OnlyOneFile = TRUE;
-	_tcscpy(szSrcPath,szSrcDirPath);
-	/*check to see if there is an ending slash, if not add one*/
-	if(szSrcPath[_tcslen(szSrcPath) -  1] != _T('\\'))
-		_tcscat (szSrcPath, _T("\\"));
-	_tcscat(szSrcPath,findBuffer.cFileName);
-	TRACE ("Source Path: %s\n", debugstr_aw(szSrcPath));
 	/* check if there can be found files as files have first priority */
-	if (IsExistingFile(szSrcPath)) dwMoveStatusFlags |= MOVE_SOURCE_IS_FILE;
-	else dwMoveStatusFlags |= MOVE_SOURCE_IS_DIR;
+	if (findBuffer.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+		dwMoveStatusFlags |= MOVE_SOURCE_IS_DIR;
+	else
+		dwMoveStatusFlags |= MOVE_SOURCE_IS_FILE;
 	while(OnlyOneFile && FindNextFile(hFile,&findBuffer))
 	{
-		_tcscpy(szSrcPath,szSrcDirPath);
-		if(szSrcPath[_tcslen(szSrcPath) -  1] != _T('\\'))
-			_tcscat (szSrcPath, _T("\\"));
-		_tcscat(szSrcPath,findBuffer.cFileName);
-		if (IsExistingFile(szSrcPath))
+		if (!(findBuffer.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
 		{
 			ConOutPrintf(_T(""));
 			if (dwMoveStatusFlags & MOVE_SOURCE_IS_FILE) OnlyOneFile = FALSE;
@@ -258,10 +246,10 @@
 	TRACE ("Do we have only one file: %s\n", OnlyOneFile ? "TRUE" :
"FALSE");
 
 	/* we have to start again to be sure we don't miss any files or folders*/
-	hFile = FindFirstFile (arg[argc - 2], &findBuffer);
+	hFile = FindFirstFile (arg[i], &findBuffer);
 	if (hFile == INVALID_HANDLE_VALUE)
 	{
-		ErrorMessage (GetLastError (), arg[argc - 2]);
+		ErrorMessage (GetLastError (), arg[i]);
 		freep (arg);
 		return 1;
 		
@@ -284,7 +272,7 @@
 	}
 	
 	/* check if source and destination paths are on different volumes */
-	if (szSrcPath[0] != szDestPath[0])
+	if (szSrcDirPath[0] != szDestPath[0])
 		dwMoveStatusFlags |= MOVE_PATHS_ON_DIF_VOL;
 	
 	/* move it */
@@ -378,7 +366,7 @@
 			!OnlyOneFile)
 		{
 			/*source has many files but there is only one destination file*/
-			error_invalid_parameter_format(arg[argc - 1]);
+			error_invalid_parameter_format(pszDest);
 			FindClose(hFile);
 			freep (arg);
 			return 1;