[ros-diffs] [hbelusca] 67055: [CMD] - Fix some comments that were otherwise hard to understand. - Don't hardcode MAX_PATH in some API calls but instead use the real array char size. - Don't leak find-file hand...

Author: hbelusca
Date: Sun Apr  5 00:39:06 2015
New Revision: 67055

URL: http://svn.reactos.org/svn/reactos?rev=67055&view=rev
Log:
[CMD]
- Fix some comments that were otherwise hard to understand.
- Don't hardcode MAX_PATH in some API calls but instead use the real array char size.
- Don't leak find-file handles: fix leaked handles to NUL pseudo-file in situations
where we do: copy some_file+NUL some_dest_file . Should fix some cmd winetests.

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

Modified: trunk/reactos/base/shell/cmd/copy.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/shell/cmd/copy.c?rev=…
==============================================================================
--- trunk/reactos/base/shell/cmd/copy.c	[iso-8859-1] (original)
+++ trunk/reactos/base/shell/cmd/copy.c	[iso-8859-1] Sun Apr  5 00:39:06 2015
@@ -341,14 +341,12 @@
     WIN32_FIND_DATA findBuffer;
     HANDLE hFile = NULL;
     BOOL bTouch = FALSE;
-    /* Used when something like "copy c*.exe d*.exe" during the process of
-       figuring out the new name */
     /* Pointer to keep track of how far through the append input(file1+file2+file3) we
are */
     TCHAR  * appendPointer = _T("\0");
     /* The full path to src and dest.  This has drive letter, folders, and filename */
     TCHAR tmpDestPath[MAX_PATH];
     TCHAR tmpSrcPath[MAX_PATH];
-    /* A bool on weather or not the destination name will be taking from the input */
+    /* A bool to know whether or not the destination name will be taken from the input
*/
     BOOL bSrcName = FALSE;
     /* Seems like a waste but it is a pointer used to copy from input to PreserveName */
     TCHAR * UseThisName;
@@ -357,6 +355,7 @@
     int size;
     TCHAR * szTouch;
     BOOL bHasWildcard, bDone = FALSE, bMoreFiles = FALSE;
+    /* Used for something like "copy c*.exe d*.exe" */
     BOOL bMultipleSource = FALSE, bMultipleDest = FALSE;
 
 
@@ -369,7 +368,7 @@
 
     nErrorLevel = 0;
 
-    /* Get the envor value if it exists */
+    /* Get the env variable value if it exists */
     evar = cmd_alloc(512 * sizeof(TCHAR));
     if (evar == NULL)
         size = 0;
@@ -578,7 +577,7 @@
         bMultipleSource = TRUE;
     }
 
-    /* Reusing the number of files variable */
+    /* Reuse the number of files variable */
     nFiles = 0;
 
     /* Check if no destination argument is passed */
@@ -586,19 +585,19 @@
     {
         /* If no destination was entered then just use
         the current directory as the destination */
-        GetCurrentDirectory(MAX_PATH, szDestPath);
+        GetCurrentDirectory(ARRAYSIZE(szDestPath), szDestPath);
     }
     else
     {
         /* Check if the destination is 'x:' */
         if ((arg[nDes][1] == _T(':')) && (arg[nDes][2] ==
_T('\0')))
         {
-            GetRootPath(arg[nDes], szDestPath, MAX_PATH);
+            GetRootPath(arg[nDes], szDestPath, ARRAYSIZE(szDestPath));
         }
         else
         {
             /* If the user entered two file names then form the full string path */
-            GetFullPathName(arg[nDes], MAX_PATH, szDestPath, NULL);
+            GetFullPathName(arg[nDes], ARRAYSIZE(szDestPath), szDestPath, NULL);
         }
 
         /* Make sure there is an ending slash to the path if the dest is a folder */
@@ -618,12 +617,12 @@
         }
     }
 
-    if (nDes != -1) /* you can only append files when there is a destination */
+    if (nDes != -1) /* Append files only when there is a destination */
     {
         if (bMultipleSource && !bMultipleDest)
         {
             /* We have multiple source files, but not multiple destination
-               files. This means we are appending the soruce files. */
+               files. This means we are appending the source files. */
             bAppend = TRUE;
             if (_tcschr(arg[nSrc], _T('|')) != NULL)
                 appendPointer = arg[nSrc];
@@ -662,7 +661,7 @@
             szSrcPath[0] = _T('\0');
 
             /* Loop through the source file name and copy all
-            the chars one at a time until it gets too + */
+               the chars one at a time until we reach the separator */
             while(TRUE)
             {
                 if (appendPointer[0] == _T('|'))
@@ -685,7 +684,7 @@
             {
                 /* Only time there is a , in the source is when they are using touch
                    Cant have a destination and can only have on ,, at the end of the
string
-                    Cant have more then one file name */
+                   Cant have more than one file name */
                 szTouch = _tcsstr(arg[nSrc], _T("|"));
                 if (_tcsncmp(szTouch,_T("|,,\0"), 4) || (nDes != -1))
                 {
@@ -714,8 +713,8 @@
         }
 
 
-        /* From this point on, we can assume that the shortest path is 3 letters long
-        and that would be [DriveLetter]:\ */
+        /* From this point on, we can assume that the shortest path is
+           3 letters long and that would be [DriveLetter]:\ */
 
         /* Check if the path has a wildcard */
         bHasWildcard = (_tcschr(szSrcPath, _T('*')) != NULL);
@@ -742,10 +741,10 @@
         /* Get a list of all the files */
         hFile = FindFirstFile(szSrcPath, &findBuffer);
 
-        /* If it couldnt open the file handle, print out the error */
+        /* If we could not open the file handle, print out the error */
         if (hFile == INVALID_HANDLE_VALUE)
         {
-            /* only print source name when more then one file */
+            /* only print source name when more than one file */
             if (bMultipleSource)
                 ConOutPrintf(_T("%s\n"), szSrcPath);
 
@@ -757,10 +756,12 @@
 
         /* Strip the paths back to the folder they are in */
         for (i = (_tcslen(szSrcPath) -  1); i > -1; i--)
+        {
             if (szSrcPath[i] != _T('\\'))
                 szSrcPath[i] = _T('\0');
             else
                 break;
+        }
 
         do
         {
@@ -803,7 +804,7 @@
                 _tcscat(tmpDestPath, findBuffer.cFileName);
             else
             {
-                /* If there is no wildcard you can use the name the user entered */
+                /* If there is no wildcard, use the name the user entered */
                 if ((_tcschr(UseThisName, _T('*')) == NULL) &&
                     (_tcschr(UseThisName, _T('?')) == NULL))
                 {
@@ -836,7 +837,7 @@
                 break;
             }
 
-            /* only print source name when more then one file */
+            /* only print source name when more than one file */
             if (bMultipleSource)
                 ConOutPrintf(_T("%s\n"), tmpSrcPath);
 
@@ -848,11 +849,10 @@
             if (nOverwrite == PROMPT_ALL || (nOverwrite == PROMPT_YES &&
bAppend))
                 dwFlags |= COPY_NO_PROMPT;
 
-            /* Tell weather the copy was successful or not */
+            /* Tell whether the copy was successful or not */
             if (copy(tmpSrcPath,tmpDestPath, bAppend, dwFlags, bTouch))
             {
                 nFiles++;
-                //LoadString(CMD_ModuleHandle, STRING_MOVE_ERROR1, szMsg,
ARRAYSIZE(szMsg));
             }
             else
             {
@@ -865,14 +865,14 @@
         /* Loop through all wildcard files */
         } while (FindNextFile(hFile, &findBuffer));
 
+        FindClose(hFile);
+
     /* Loop through all files in src string with a + */
-    } while(!bDone);
+    } while (!bDone);
 
     /* print out the number of files copied */
     ConOutResPrintf(STRING_COPY_FILE, bAppend ? 1 : nFiles);
 
-    if (hFile) FindClose(hFile);
-
     if (arg != NULL)
         freep(arg);