Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: svn commit: r1352385 - in /hadoop/common/branches/branch-1-win:
 CHANGES.txt
 src/winutils/chmod.c
Date: Thu, 21 Jun 2012 01:16:16 -0000
To: common-commits@hadoop.apache.org
From: sradia@apache.org
Message-Id: <20120621011616.75BFB2388A32@eris.apache.org>

Author: sradia
Date: Thu Jun 21 01:16:15 2012
New Revision: 1352385

URL: http://svn.apache.org/viewvc?rev=1352385&view=rev
Log:
HADOOP-8454 Fix the ‘chmod =[perm]’ bug in winutils (Chuan Liu via sanjay)

Modified:
    hadoop/common/branches/branch-1-win/CHANGES.txt
    hadoop/common/branches/branch-1-win/src/winutils/chmod.c

Modified: hadoop/common/branches/branch-1-win/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.txt?rev=1352385&r1=1352384&r2=1352385&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/CHANGES.txt (original)
+++ hadoop/common/branches/branch-1-win/CHANGES.txt Thu Jun 21 01:16:15 2012
@@ -37,6 +37,7 @@ branch-hadoop-1-win - unreleased
 
     HADOOP-8409 Fix TestCommandLineJobSubmission and TestGenericOptionsParser to work for windows (Ivan Mitic via Sanjay Radia) 
 
+    HADOOP-8454 Fix the âchmod =[perm]â bug in winutils (Chuan Liu via sanjay)
 
 Release 1.1.0 - unreleased
 

Modified: hadoop/common/branches/branch-1-win/src/winutils/chmod.c
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/winutils/chmod.c?rev=1352385&r1=1352384&r2=1352385&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/winutils/chmod.c (original)
+++ hadoop/common/branches/branch-1-win/src/winutils/chmod.c Thu Jun 21 01:16:15 2012
@@ -102,10 +102,10 @@ static USHORT ComputeNewMode(__in USHORT
 // Function: Chmod
 //
 // Description:
-//	The main method for chmod command
+//  The main method for chmod command
 //
 // Returns:
-//	0: on success
+//  0: on success
 //
 // Notes:
 //
@@ -169,11 +169,11 @@ ChmodEnd:
 // Function: ChangeFileMode
 //
 // Description:
-//	Wrapper function for change file mode. Choose either change by action or by
+//  Wrapper function for change file mode. Choose either change by action or by
 //  access mask.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
@@ -191,10 +191,10 @@ static BOOL ChangeFileMode(__in LPCWSTR 
 // Function: ChangeFileModeRecursively
 //
 // Description:
-//	Travel the directory recursively to change the permissions.
+//  Travel the directory recursively to change the permissions.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
@@ -235,10 +235,7 @@ static BOOL ChangeFileModeRecursively(__
      }
   }
 
-  // MAX_PATH is used here, because we use relative path, and relative
-  // paths are always limited to a total of MAX_PATH characters.
-  //
-  if (FAILED(StringCchLengthW(path, MAX_PATH - 3, &pathSize)))
+  if (FAILED(StringCchLengthW(path, STRSAFE_MAX_CCH - 3, &pathSize)))
   {
     return FALSE;
   }
@@ -315,10 +312,10 @@ ChangeFileModeRecursivelyEnd:
 // Function: ChangeFileModeByMask
 //
 // Description:
-//	Change a file or direcotry at the path to Unix mode
+//  Change a file or direcotry at the path to Unix mode
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
@@ -466,10 +463,10 @@ ChangeFileMode:
 // Function: ParseCommandLineArguments
 //
 // Description:
-//	Parse command line arguments for chmod.
+//  Parse command line arguments for chmod.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
@@ -539,14 +536,14 @@ static BOOL ParseCommandLineArguments(__
 // Function: FreeActions
 //
 // Description:
-//	Free a linked list of mode change actions given the head node.
+//  Free a linked list of mode change actions given the head node.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
-//	none
+//  none
 //
 static BOOL FreeActions(PMODE_CHANGE_ACTION actions)
 {
@@ -576,13 +573,13 @@ static BOOL FreeActions(PMODE_CHANGE_ACT
 // Function: ComputeNewMode
 //
 // Description:
-//	Compute a new mode based on the old mode and a mode change action.
+//  Compute a new mode based on the old mode and a mode change action.
 //
 // Returns:
-//	The newly computed mode
+//  The newly computed mode
 //
 // Notes:
-//	Apply 'rwx' permission mask or reference permission mode according to the
+//  Apply 'rwx' permission mask or reference permission mode according to the
 //  '+', '-', or '=' operator.
 //
 static USHORT ComputeNewMode(__in USHORT oldMode,
@@ -596,11 +593,14 @@ static USHORT ComputeNewMode(__in USHORT
   USHORT mask = 0;
   USHORT mode = 0;
 
-  // Operation and reference mode are exclusive
+  // Operations are exclusive
   //
   assert(op == CHMOD_OP_EQUAL || op == CHMOD_OP_PLUS || op == CHMOD_OP_MINUS);
-  assert(ref == CHMOD_WHO_GROUP || ref == CHMOD_WHO_USER ||
-    ref == CHMOD_WHO_OTHER);
+
+  // We should have only permissions or a reference target, not both.
+  //
+  assert((perm != CHMOD_PERM_NA && ref == CHMOD_WHO_NONE) ||
+    (perm == CHMOD_PERM_NA && ref != CHMOD_WHO_NONE));
 
   if(perm == CHMOD_PERM_NA && ref == CHMOD_WHO_NONE)
   {
@@ -631,14 +631,34 @@ static USHORT ComputeNewMode(__in USHORT
         mask |= exeMask;
     }
   }
-
-  mask |= oldMode & ref;
+  else if (ref != CHMOD_WHO_NONE)
+  {
+    mask |= oldMode & ref;
+    switch(ref)
+    {
+    case CHMOD_WHO_GROUP:
+      mask |= mask >> 3;
+      mask |= mask << 3;
+      break;
+    case CHMOD_WHO_OTHER:
+      mask |= mask << 3;
+      mask |= mask << 6;
+      break;
+    case CHMOD_WHO_USER:
+      mask |= mask >> 3;
+      mask |= mask >> 6;
+      break;
+    default:
+      // Reference modes can only be U/G/O and are exclusive
+      assert(FALSE);
+    }
+  }
 
   mask &= who;
 
   if (op == CHMOD_OP_EQUAL)
   {
-    mode = mask;
+    mode = (oldMode & (~who)) | mask;
   }
   else if (op == CHMOD_OP_MINUS)
   {
@@ -656,15 +676,15 @@ static USHORT ComputeNewMode(__in USHORT
 // Function: ConvertActionsToMask
 //
 // Description:
-//	Convert a linked list of mode change actions to the Unix permission mask
+//  Convert a linked list of mode change actions to the Unix permission mask
 //  given the head node.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
-//	none
+//  none
 //
 static BOOL ConvertActionsToMask(__in LPCWSTR path,
   __in PMODE_CHANGE_ACTION actions, __out PUSHORT puMask)
@@ -712,14 +732,14 @@ static BOOL ConvertActionsToMask(__in LP
 // Function: ChangeFileModeByActions
 //
 // Description:
-//	Change a file mode through a list of actions.
+//  Change a file mode through a list of actions.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
-//	none
+//  none
 //
 static BOOL ChangeFileModeByActions(__in LPCWSTR path,
   PMODE_CHANGE_ACTION actions)
@@ -736,14 +756,14 @@ static BOOL ChangeFileModeByActions(__in
 // Function: ParseMode
 //
 // Description:
-//	Convert a mode string into a linked list of actions
+//  Convert a mode string into a linked list of actions
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
-//	Take a state machine approach to parse the mode. Each mode change action
+//  Take a state machine approach to parse the mode. Each mode change action
 //  will be a node in the output linked list. The state machine has five state,
 //  and each will only transit to the next; the end state can transit back to
 //  the first state, and thus form a circle. In each state, if we see a
@@ -929,13 +949,13 @@ static BOOL ParseMode(LPCWSTR modeString
 // Function: ParseOctalMode
 //
 // Description:
-//	Convert the 3 or 4 digits Unix mask string into the binary representation
+//  Convert the 3 or 4 digits Unix mask string into the binary representation
 //  of the Unix access mask, i.e. 9 bits each an indicator of the permission
 //  of 'rwxrwxrwx', i.e. user's, group's, and owner's read, write, and
 //  execute/search permissions.
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
@@ -954,7 +974,7 @@ static BOOL ParseOctalMode(LPCWSTR tsMas
   if (FAILED(StringCchLengthW(tsMask, STRSAFE_MAX_CCH, &tsMaskLen)))
     return FALSE;
 
-  if (tsMaskLen != 4 && tsMaskLen != 3)
+  if (tsMaskLen == 0 || tsMaskLen > 4)
   {
     return FALSE;
   }
@@ -989,14 +1009,14 @@ static BOOL ParseOctalMode(LPCWSTR tsMas
 // Function: GetWindowsAccessMask
 //
 // Description:
-//	Get the Windows AccessMask for user, group and everyone based on the Unix
+//  Get the Windows AccessMask for user, group and everyone based on the Unix
 //  permission mask
 //
 // Returns:
-//	none
+//  none
 //
 // Notes:
-//	none
+//  none
 //
 static void GetWindowsAccessMask(USHORT unixMask,
   ACCESS_MASK *userAllow,
@@ -1073,14 +1093,14 @@ static void GetWindowsAccessMask(USHORT 
 // Function: GetWindowsDACLs
 //
 // Description:
-//	Get the Windows DACs based the Unix access mask
+//  Get the Windows DACs based the Unix access mask
 //
 // Returns:
-//	TRUE: on success
+//  TRUE: on success
 //  FALSE: otherwise
 //
 // Notes:
-//	none
+//  none
 //
 static BOOL GetWindowsDACLs(__in USHORT unixMask,
   __in PSID pOwnerSid, __in PSID pGroupSid, __out PACL *ppNewDACL)