[ATTRIB] Improve command-line parsing. (#5288)

Fix arguments parsing and redundant code in case no file specification
has been given.
(Handles both "attrib" and "attrib +h /s" as given in example.)

base/applications/cmdutils/attrib/attrib.c

@@ -284,41 +284,50 @@ ChangeAttribute(
 int wmain(int argc, WCHAR *argv[])
 {
     INT i;
-    WCHAR  szPath[MAX_PATH] = L""; // For case we only use 'attrib +h /s' there is no szPath
-    WCHAR  szFileName [MAX_PATH];
     BOOL bRecurse = FALSE;
     BOOL bDirectories = FALSE;
     DWORD dwAttrib = 0;
     DWORD dwMask = 0;
-    LPWSTR p;
+    BOOL bFound = FALSE;
+    PWSTR pszFileName;
+    WCHAR szFilePath[MAX_PATH + 2] = L""; // + 2 to reserve an extra path separator and a NULL-terminator.
 
     /* Initialize the Console Standard Streams */
     ConInitStdStreams();
 
-    /* Print help */
+    /* Check for options and file specifications */
-    if (argc > 1 && wcscmp(argv[1], L"/?") == 0)
+    for (i = 1; i < argc; i++)
+    {
+        if (*argv[i] == L'/')
+        {
+            /* Print help and bail out if needed */
+            if (wcscmp(argv[i], L"/?") == 0)
             {
                 ConResPuts(StdOut, STRING_ATTRIB_HELP);
                 return 0;
             }
-
+            else
-    /* check for options */
+            /* Retrieve the enumeration modes */
-    for (i = 1; i < argc; i++)
-    {
             if (wcsicmp(argv[i], L"/s") == 0)
                 bRecurse = TRUE;
             else if (wcsicmp(argv[i], L"/d") == 0)
                 bDirectories = TRUE;
+            else
+            {
+                /* Unknown option */
+                ConResPrintf(StdErr, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
+                return -1;
             }
+        }
+        else
+        /* Build attributes and mask */
+        if ((*argv[i] == L'+') || (*argv[i] == L'-'))
+        {
+            BOOL bAdd = (*argv[i] == L'+');
 
-    /* create attributes and mask */
-    for (i = 1; i < argc; i++)
-    {
-        if (*argv[i] == L'+')
-        {
             if (wcslen(argv[i]) != 2)
             {
-                ConResPrintf(StdOut, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
+                ConResPrintf(StdErr, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
                 return -1;
             }
 
@@ -326,120 +335,98 @@ int wmain(int argc, WCHAR *argv[])
             {
                 case L'A':
                     dwMask |= FILE_ATTRIBUTE_ARCHIVE;
+                    if (bAdd)
                         dwAttrib |= FILE_ATTRIBUTE_ARCHIVE;
-                    break;
+                    else
-
+                        dwAttrib &= ~FILE_ATTRIBUTE_ARCHIVE;
-                case L'H':
-                    dwMask   |= FILE_ATTRIBUTE_HIDDEN;
-                    dwAttrib |= FILE_ATTRIBUTE_HIDDEN;
-                    break;
-
-                case L'R':
-                    dwMask   |= FILE_ATTRIBUTE_READONLY;
-                    dwAttrib |= FILE_ATTRIBUTE_READONLY;
                     break;
 
                 case L'S':
                     dwMask |= FILE_ATTRIBUTE_SYSTEM;
+                    if (bAdd)
                         dwAttrib |= FILE_ATTRIBUTE_SYSTEM;
-                    break;
+                    else
-
+                        dwAttrib &= ~FILE_ATTRIBUTE_SYSTEM;
-                default:
-                    ConResPrintf(StdOut, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
-                    return -1;
-            }
-        }
-        else if (*argv[i] == L'-')
-        {
-            if (wcslen(argv[i]) != 2)
-            {
-                ConResPrintf(StdOut, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
-                return -1;
-            }
-
-            switch (towupper(argv[i][1]))
-            {
-                case L'A':
-                    dwMask   |= FILE_ATTRIBUTE_ARCHIVE;
-                    dwAttrib &= ~FILE_ATTRIBUTE_ARCHIVE;
                     break;
 
                 case L'H':
                     dwMask |= FILE_ATTRIBUTE_HIDDEN;
+                    if (bAdd)
+                        dwAttrib |= FILE_ATTRIBUTE_HIDDEN;
+                    else
                         dwAttrib &= ~FILE_ATTRIBUTE_HIDDEN;
                     break;
 
                 case L'R':
                     dwMask |= FILE_ATTRIBUTE_READONLY;
+                    if (bAdd)
+                        dwAttrib |= FILE_ATTRIBUTE_READONLY;
+                    else
                         dwAttrib &= ~FILE_ATTRIBUTE_READONLY;
                     break;
 
-                case L'S':
-                    dwMask   |= FILE_ATTRIBUTE_SYSTEM;
-                    dwAttrib &= ~FILE_ATTRIBUTE_SYSTEM;
-                    break;
-
                 default:
-                    ConResPrintf(StdOut, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
+                    ConResPrintf(StdErr, STRING_ERROR_INVALID_PARAM_FORMAT, argv[i]);
                     return -1;
             }
         }
+        else
+        {
+            /* At least one file specification found */
+            bFound = TRUE;
+        }
     }
 
-    if (argc == 1)
+    /* If no file specification was found, operate on all files of the current directory */
+    if (!bFound)
     {
-        DWORD len;
+        DWORD len = GetCurrentDirectoryW(_countof(szFilePath) - 2, szFilePath);
-
+        if (szFilePath[len - 1] != L'\\')
-        len = GetCurrentDirectory(MAX_PATH, szPath);
-        if (szPath[len-1] != L'\\')
         {
-            szPath[len] = L'\\';
+            szFilePath[len] = L'\\';
-            szPath[len + 1] = UNICODE_NULL;
+            szFilePath[len + 1] = UNICODE_NULL;
         }
-        wcscpy(szFileName, L"*.*");
+        pszFileName = L"*.*";
-        PrintAttribute(szPath, szFileName, bRecurse, bDirectories);
-        return 0;
-    }
-
-    /* get full file name */
-    for (i = 1; i < argc; i++)
-    {
-        if (*argv[i] == L'+' || *argv[i] == L'-' || *argv[i] == L'/')
-            continue;
-
-        GetFullPathNameW(argv[i], MAX_PATH, szPath, &p);
-        wcscpy(szFileName, p);
-        *p = 0;
 
         if (dwMask == 0)
+            bFound = PrintAttribute(szFilePath, pszFileName, bRecurse, bDirectories);
+        else
+            bFound = ChangeAttribute(szFilePath, pszFileName, bRecurse, bDirectories, dwMask, dwAttrib);
+
+        if (!bFound)
+            ConResPrintf(StdOut, STRING_FILE_NOT_FOUND, pszFileName);
+
+        return 0;
+    }
+
+    /* Operate on each file specification */
+    for (i = 1; i < argc; i++)
     {
-            if (!PrintAttribute(szPath, szFileName, bRecurse, bDirectories))
+        /* Skip options */
+        if (*argv[i] == L'/' || *argv[i] == L'+' || *argv[i] == L'-')
+            continue;
+
+        GetFullPathNameW(argv[i], _countof(szFilePath) - 2, szFilePath, &pszFileName);
+        if (pszFileName)
         {
+            /* Move the file part so as to separate and NULL-terminate the directory */
+            MoveMemory(pszFileName + 1, pszFileName,
+                       sizeof(szFilePath) - (pszFileName -szFilePath + 1) * sizeof(*szFilePath));
+            *pszFileName++ = UNICODE_NULL;
+        }
+        else
+        {
+            pszFileName = L"";
+        }
+
+        if (dwMask == 0)
+            bFound = PrintAttribute(szFilePath, pszFileName, bRecurse, bDirectories);
+        else
+            bFound = ChangeAttribute(szFilePath, pszFileName, bRecurse, bDirectories, dwMask, dwAttrib);
+
+        if (!bFound)
             ConResPrintf(StdOut, STRING_FILE_NOT_FOUND, argv[i]);
     }
-        }
-        else if (!ChangeAttribute(szPath, szFileName, bRecurse, bDirectories, dwMask, dwAttrib))
-        {
-            ConResPrintf(StdOut, STRING_FILE_NOT_FOUND, argv[i]);
-        }
-    }
-
-// Code below handles the special case of 'attrib +h /s' and similar
-
-    if (bRecurse && dwMask && (wcscmp(szPath, L"") == 0))
-    {
-        DWORD len;
-
-        len = GetCurrentDirectory(MAX_PATH, szPath);
-        if (szPath[len-1] != L'\\')
-        {
-            szPath[len] = L'\\';
-            szPath[len + 1] = UNICODE_NULL;
-        }
-        wcscpy(szFileName, L"*.*");
-        if (!ChangeAttribute(szPath, szFileName, bRecurse, bDirectories, dwMask, dwAttrib))
-            ConResPrintf(StdOut, STRING_FILE_NOT_FOUND, szFileName);
-    }
 
     return 0;
 }