[ros-diffs] [ion] 57478: [RTL]: Cleanup, comment, and fix SID functions. For example, RtlEqualPrefixSid actually checks only the prefix, not the entire SID.

Author: ion
Date: Thu Oct  4 16:55:53 2012
New Revision: 57478

URL: http://svn.reactos.org/svn/reactos?rev=57478&view=rev
Log:
[RTL]: Cleanup, comment, and fix SID functions. For example, RtlEqualPrefixSid actually
checks only the prefix, not the entire SID.

Modified:
    trunk/reactos/lib/rtl/access.c
    trunk/reactos/lib/rtl/sid.c

Modified: trunk/reactos/lib/rtl/access.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/access.c?rev=57478…
==============================================================================
--- trunk/reactos/lib/rtl/access.c [iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/access.c [iso-8859-1] Thu Oct  4 16:55:53 2012
@@ -8,7 +8,6 @@
 /* INCLUDES *****************************************************************/
 
 #include <rtl.h>
-
 #define NDEBUG
 #include <debug.h>
 
@@ -19,11 +18,13 @@
  */
 BOOLEAN
 NTAPI
-RtlAreAllAccessesGranted(ACCESS_MASK GrantedAccess,
-                         ACCESS_MASK DesiredAccess)
+RtlAreAllAccessesGranted(IN ACCESS_MASK GrantedAccess,
+                         IN ACCESS_MASK DesiredAccess)
 {
-  PAGED_CODE_RTL();
-  return ((GrantedAccess & DesiredAccess) == DesiredAccess);
+    PAGED_CODE_RTL();
+
+    /* Return if there's no leftover bits after granting all of them */
+    return !(~GrantedAccess & DesiredAccess);
 }
 
 /*
@@ -31,11 +32,13 @@
  */
 BOOLEAN
 NTAPI
-RtlAreAnyAccessesGranted(ACCESS_MASK GrantedAccess,
-                         ACCESS_MASK DesiredAccess)
+RtlAreAnyAccessesGranted(IN ACCESS_MASK GrantedAccess,
+                         IN ACCESS_MASK DesiredAccess)
 {
     PAGED_CODE_RTL();
-    return ((GrantedAccess & DesiredAccess) != 0);
+
+    /* Return if there's any leftover bits after granting all of them */
+    return (GrantedAccess & DesiredAccess);
 }
 
 /*
@@ -43,19 +46,18 @@
  */
 VOID
 NTAPI
-RtlMapGenericMask(PACCESS_MASK AccessMask,
-                  PGENERIC_MAPPING GenericMapping)
+RtlMapGenericMask(IN OUT PACCESS_MASK AccessMask,
+                  IN PGENERIC_MAPPING GenericMapping)
 {
     PAGED_CODE_RTL();
 
+    /* Convert mappings */
     if (*AccessMask & GENERIC_READ) *AccessMask |= GenericMapping->GenericRead;
-
     if (*AccessMask & GENERIC_WRITE) *AccessMask |= GenericMapping->GenericWrite;
-
     if (*AccessMask & GENERIC_EXECUTE) *AccessMask |=
GenericMapping->GenericExecute;
-
     if (*AccessMask & GENERIC_ALL) *AccessMask |= GenericMapping->GenericAll;
 
+    /* Clear generic flags */
     *AccessMask &= ~(GENERIC_READ | GENERIC_WRITE | GENERIC_EXECUTE | GENERIC_ALL);
 }
 

Modified: trunk/reactos/lib/rtl/sid.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/sid.c?rev=57478&am…
==============================================================================
--- trunk/reactos/lib/rtl/sid.c [iso-8859-1] (original)
+++ trunk/reactos/lib/rtl/sid.c [iso-8859-1] Thu Oct  4 16:55:53 2012
@@ -9,7 +9,6 @@
 /* INCLUDES *****************************************************************/
 
 #include <rtl.h>
-
 #define NDEBUG
 #include <debug.h>
 
@@ -22,19 +21,31 @@
 RtlValidSid(IN PSID Sid_)
 {
     PISID Sid = Sid_;
-
-    PAGED_CODE_RTL();
-
-    if ((Sid->Revision != SID_REVISION) ||
-        (Sid->SubAuthorityCount > SID_MAX_SUB_AUTHORITIES))
-    {
+    PAGED_CODE_RTL();
+
+    /* Use SEH in case any pointer is invalid */
+    _SEH2_TRY
+    {
+        /* Validate the revision and subauthority count */
+        if ((Sid) &&
+            (((Sid->Revision & 0xF) != SID_REVISION) ||
+             (Sid->SubAuthorityCount > SID_MAX_SUB_AUTHORITIES)))
+        {
+            /* It's not, fail */
+            return FALSE;
+        }
+    }
+    _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+    {
+        /* Access violation, SID is not valid */
         return FALSE;
     }
-
+    _SEH2_END;
+
+    /* All good */
     return TRUE;
 }
 
-
 /*
  * @implemented
  */
@@ -44,10 +55,10 @@
 {
     PAGED_CODE_RTL();
 
+    /* Return the required length */
     return (ULONG)FIELD_OFFSET(SID,
                                SubAuthority[SubAuthorityCount]);
 }
-
 
 /*
  * @implemented
@@ -59,18 +70,16 @@
                  IN UCHAR SubAuthorityCount)
 {
     PISID Sid = Sid_;
-
-    PAGED_CODE_RTL();
-
+    PAGED_CODE_RTL();
+
+    /* Fill out the header */
     Sid->Revision = SID_REVISION;
     Sid->SubAuthorityCount = SubAuthorityCount;
-    memcpy(&Sid->IdentifierAuthority,
-           IdentifierAuthority,
-           sizeof(SID_IDENTIFIER_AUTHORITY));
-
+    Sid->IdentifierAuthority = *IdentifierAuthority;
+
+    /* All good */
     return STATUS_SUCCESS;
 }
-
 
 /*
  * @implemented
@@ -81,13 +90,12 @@
                    IN ULONG SubAuthority)
 {
     PISID Sid = Sid_;
-
-    PAGED_CODE_RTL();
-
+    PAGED_CODE_RTL();
+
+    /* Return the offset */
     return (PULONG)&Sid->SubAuthority[SubAuthority];
 }
 
-
 /*
  * @implemented
  */
@@ -96,12 +104,25 @@
 RtlSubAuthorityCountSid(IN PSID Sid_)
 {
     PISID Sid =  Sid_;
-
-    PAGED_CODE_RTL();
-
+    PAGED_CODE_RTL();
+
+    /* Return the offset to the count */
     return &Sid->SubAuthorityCount;
 }
 
+/*
+ * @implemented
+ */
+PSID_IDENTIFIER_AUTHORITY
+NTAPI
+RtlIdentifierAuthoritySid(IN PSID Sid_)
+{
+    PISID Sid =  Sid_;
+    PAGED_CODE_RTL();
+
+    /* Return the offset to the identifier authority */
+    return &Sid->IdentifierAuthority;
+}
 
 /*
  * @implemented
@@ -111,22 +132,15 @@
 RtlEqualSid(IN PSID Sid1_,
             IN PSID Sid2_)
 {
-    PISID Sid1 = Sid1_;
-    PISID Sid2 = Sid2_;
-    SIZE_T SidLen;
-
-    PAGED_CODE_RTL();
-
-    if (Sid1->Revision != Sid2->Revision ||
-        (*RtlSubAuthorityCountSid(Sid1)) != (*RtlSubAuthorityCountSid(Sid2)))
-    {
-        return FALSE;
-    }
-
-     SidLen = RtlLengthSid(Sid1);
-     return RtlCompareMemory(Sid1, Sid2, SidLen) == SidLen;
-}
-
+    PISID Sid1 = Sid1_, Sid2 = Sid2_;
+    PAGED_CODE_RTL();
+
+    /* Quick compare of the revision and the count */
+    if (*(PUSHORT)&Sid1->Revision != *(PUSHORT)&Sid2->Revision) return
FALSE;
+
+    /* Get the length and compare it the long way */
+    return RtlEqualMemory(Sid1, Sid2, RtlLengthSid(Sid1));
+}
 
 /*
  * @implemented
@@ -136,233 +150,213 @@
 RtlLengthSid(IN PSID Sid_)
 {
     PISID Sid = Sid_;
-
-    PAGED_CODE_RTL();
-
+    PAGED_CODE_RTL();
+
+    /* The offset to the last index + 1 (since it's a count) is the length */
     return (ULONG)FIELD_OFFSET(SID,
                                SubAuthority[Sid->SubAuthorityCount]);
 }
 
-
 /*
  * @implemented
  */
 NTSTATUS
 NTAPI
-RtlCopySid(ULONG BufferLength,
-           PSID Dest,
-           PSID Src)
-{
-    PAGED_CODE_RTL();
-
-    if (BufferLength < RtlLengthSid(Src))
-    {
-        return STATUS_UNSUCCESSFUL;
-    }
-
-    memmove(Dest,
-            Src,
-            RtlLengthSid(Src));
-
+RtlCopySid(IN ULONG BufferLength,
+           IN PSID Dest,
+           IN PSID Src)
+{
+    ULONG SidLength;
+    PAGED_CODE_RTL();
+
+    /* Make sure the buffer is large enough*/
+    SidLength = RtlLengthSid(Src);
+    if (SidLength > BufferLength) return STATUS_BUFFER_TOO_SMALL;
+
+    /* And then copy the SID */
+    RtlMoveMemory(Dest, Src, SidLength);
     return STATUS_SUCCESS;
 }
 
+/*
+ * @implemented
+ */
+PVOID
+NTAPI
+RtlFreeSid(IN PSID Sid)
+{
+    PAGED_CODE_RTL();
+
+    /* Free the SID and always return NULL */
+    RtlpFreeMemory(Sid, TAG_SID);
+    return NULL;
+}
+
+/*
+ * @implemented
+ */
+BOOLEAN
+NTAPI
+RtlEqualPrefixSid(IN PSID Sid1_,
+                  IN PSID Sid2_)
+{
+    PISID Sid1 = Sid1_, Sid2 = Sid2_;
+    ULONG i;
+    PAGED_CODE_RTL();
+
+    /* Revisions have to match */
+    if (Sid1->Revision != Sid2->Revision) return FALSE;
+
+    /* The identifier authorities have to match */
+    if ((Sid1->IdentifierAuthority.Value[0] == Sid2->IdentifierAuthority.Value[0])
&&
+        (Sid1->IdentifierAuthority.Value[1] == Sid2->IdentifierAuthority.Value[1])
&&
+        (Sid1->IdentifierAuthority.Value[2] == Sid2->IdentifierAuthority.Value[2])
&&
+        (Sid1->IdentifierAuthority.Value[3] == Sid2->IdentifierAuthority.Value[3])
&&
+        (Sid1->IdentifierAuthority.Value[4] == Sid2->IdentifierAuthority.Value[4])
&&
+        (Sid1->IdentifierAuthority.Value[5] ==
Sid2->IdentifierAuthority.Value[5]))
+    {
+        /* The subauthority counts have to match */
+        if (Sid1->SubAuthorityCount == Sid2->SubAuthorityCount)
+        {
+            /* If there aren't any in SID1, means none in SID2 either, so equal */
+            if (!Sid1->SubAuthorityCount) return TRUE;
+
+            /* Now compare all the subauthority values BUT the last one */
+            for (i = 0; i < (Sid1->SubAuthorityCount - 1); i++)
+            {
+                /* Does any mismatch? */
+                if (Sid1->SubAuthority[i] != Sid2->SubAuthority[i])
+                {
+                    /* Prefix doesn't match, fail */
+                    return FALSE;
+                }
+            }
+
+            /* Everything that should matches, does, return success */
+            return TRUE;
+        }
+    }
+
+    /* Identifiers don't match, fail */
+    return FALSE;
+}
 
 /*
  * @implemented
  */
 NTSTATUS
 NTAPI
-RtlCopySidAndAttributesArray(ULONG Count,
-                             PSID_AND_ATTRIBUTES Src,
-                             ULONG SidAreaSize,
-                             PSID_AND_ATTRIBUTES Dest,
-                             PVOID SidArea,
-                             PVOID* RemainingSidArea,
-                             PULONG RemainingSidAreaSize)
-{
-    ULONG SidLength;
-    ULONG Length;
-    ULONG i;
-
-    PAGED_CODE_RTL();
-
-    Length = SidAreaSize;
-
+RtlCopySidAndAttributesArray(IN ULONG Count,
+                             IN PSID_AND_ATTRIBUTES Src,
+                             IN ULONG SidAreaSize,
+                             IN PSID_AND_ATTRIBUTES Dest,
+                             IN PVOID SidArea,
+                             OUT PVOID* RemainingSidArea,
+                             OUT PULONG RemainingSidAreaSize)
+{
+    ULONG SidLength, i;
+    PAGED_CODE_RTL();
+
+    /* Loop all the attributes */
     for (i = 0; i < Count; i++)
     {
-        if (RtlLengthSid(Src[i].Sid) > Length)
-        {
-            return STATUS_BUFFER_TOO_SMALL;
-        }
-
+        /* Make sure this SID can fit in the buffer */
         SidLength = RtlLengthSid(Src[i].Sid);
-        Length = Length - SidLength;
+        if (SidLength > SidAreaSize) return STATUS_BUFFER_TOO_SMALL;
+
+        /* Consume remaining buffer space for this SID */
+        SidAreaSize -= SidLength;
+
+        /* Copy the SID and attributes */
         Dest[i].Sid = SidArea;
         Dest[i].Attributes = Src[i].Attributes;
-        RtlCopySid(SidLength,
-                   SidArea,
-                   Src[i].Sid);
+        RtlCopySid(SidLength, SidArea, Src[i].Sid);
+
+        /* Push the buffer area where the SID will reset */
         SidArea = (PVOID)((ULONG_PTR)SidArea + SidLength);
     }
 
+    /* Return how much space is left, and where the buffer is at now */
     *RemainingSidArea = SidArea;
-    *RemainingSidAreaSize = Length;
-
+    *RemainingSidAreaSize = SidAreaSize;
     return STATUS_SUCCESS;
 }
 
-
-/*
- * @implemented
- */
-PSID_IDENTIFIER_AUTHORITY
-NTAPI
-RtlIdentifierAuthoritySid(IN PSID Sid_)
-{
-    PISID Sid =  Sid_;
-
-    PAGED_CODE_RTL();
-
-    return &Sid->IdentifierAuthority;
-}
-
-
 /*
  * @implemented
  */
 NTSTATUS
 NTAPI
-RtlAllocateAndInitializeSid(PSID_IDENTIFIER_AUTHORITY IdentifierAuthority,
-                            UCHAR SubAuthorityCount,
-                            ULONG SubAuthority0,
-                            ULONG SubAuthority1,
-                            ULONG SubAuthority2,
-                            ULONG SubAuthority3,
-                            ULONG SubAuthority4,
-                            ULONG SubAuthority5,
-                            ULONG SubAuthority6,
-                            ULONG SubAuthority7,
-                            PSID *Sid)
+RtlAllocateAndInitializeSid(IN PSID_IDENTIFIER_AUTHORITY IdentifierAuthority,
+                            IN UCHAR SubAuthorityCount,
+                            IN ULONG SubAuthority0,
+                            IN ULONG SubAuthority1,
+                            IN ULONG SubAuthority2,
+                            IN ULONG SubAuthority3,
+                            IN ULONG SubAuthority4,
+                            IN ULONG SubAuthority5,
+                            IN ULONG SubAuthority6,
+                            IN ULONG SubAuthority7,
+                            OUT PSID *Sid)
 {
     PISID pSid;
-
-    PAGED_CODE_RTL();
-
-    if (SubAuthorityCount > 8)
-        return STATUS_INVALID_SID;
-
-    if (Sid == NULL)
-        return STATUS_INVALID_PARAMETER;
-
-    pSid = RtlpAllocateMemory(RtlLengthRequiredSid(SubAuthorityCount),
-                              TAG_SID);
-    if (pSid == NULL)
-        return STATUS_NO_MEMORY;
-
+    PAGED_CODE_RTL();
+
+    /* SIDs can only have up to 8 subauthorities */
+    if (SubAuthorityCount > 8) return STATUS_INVALID_SID;
+
+    /* Allocate memory to hold the SID */
+    pSid = RtlpAllocateMemory(RtlLengthRequiredSid(SubAuthorityCount), TAG_SID);
+    if (!pSid) return STATUS_NO_MEMORY;
+
+    /* Fill out the header */
     pSid->Revision = SID_REVISION;
     pSid->SubAuthorityCount = SubAuthorityCount;
-    memcpy(&pSid->IdentifierAuthority,
-           IdentifierAuthority,
-           sizeof(SID_IDENTIFIER_AUTHORITY));
-
+    pSid->IdentifierAuthority = *IdentifierAuthority;
+
+    /* Iteraratively drop into each successive lower count */
     switch (SubAuthorityCount)
     {
-        case 8:
-            pSid->SubAuthority[7] = SubAuthority7;
-        case 7:
-            pSid->SubAuthority[6] = SubAuthority6;
-        case 6:
-            pSid->SubAuthority[5] = SubAuthority5;
-        case 5:
-            pSid->SubAuthority[4] = SubAuthority4;
-        case 4:
-            pSid->SubAuthority[3] = SubAuthority3;
-        case 3:
-            pSid->SubAuthority[2] = SubAuthority2;
-        case 2:
-            pSid->SubAuthority[1] = SubAuthority1;
-        case 1:
-            pSid->SubAuthority[0] = SubAuthority0;
-            break;
-    }
-
+        /* And copy the needed subahority */
+        case 8: pSid->SubAuthority[7] = SubAuthority7;
+        case 7: pSid->SubAuthority[6] = SubAuthority6;
+        case 6: pSid->SubAuthority[5] = SubAuthority5;
+        case 5: pSid->SubAuthority[4] = SubAuthority4;
+        case 4: pSid->SubAuthority[3] = SubAuthority3;
+        case 3: pSid->SubAuthority[2] = SubAuthority2;
+        case 2: pSid->SubAuthority[1] = SubAuthority1;
+        case 1: pSid->SubAuthority[0] = SubAuthority0;
+        default: break;
+    }
+
+    /* Return the allocated SID */
     *Sid = pSid;
-
     return STATUS_SUCCESS;
 }
 
-
-/*
- * @implemented
- *
- * RETURNS
- *  Docs says FreeSid does NOT return a value
- *  even thou it's defined to return a PVOID...
- */
-PVOID
-NTAPI
-RtlFreeSid(IN PSID Sid)
-{
-    PAGED_CODE_RTL();
-
-    RtlpFreeMemory(Sid, TAG_SID);
-    return NULL;
-}
-
-
-/*
- * @implemented
- */
-BOOLEAN
-NTAPI
-RtlEqualPrefixSid(IN PSID Sid1_,
-                  IN PSID Sid2_)
-{
-    PISID Sid1 = Sid1_;
-    PISID Sid2 = Sid2_;
-    SIZE_T SidLen;
-
-    PAGED_CODE_RTL();
-
-    if (Sid1->SubAuthorityCount == Sid2->SubAuthorityCount)
-    {
-        SidLen = FIELD_OFFSET(SID,
-                              SubAuthority[Sid1->SubAuthorityCount]);
-        return RtlCompareMemory(Sid1,
-                                Sid2,
-                                SidLen) == SidLen;
-    }
-
-    return FALSE;
-}
-
-
 /*
  * @implemented
  */
 NTSTATUS
 NTAPI
-RtlConvertSidToUnicodeString(PUNICODE_STRING String,
-                             PSID Sid_,
-                             BOOLEAN AllocateBuffer)
+RtlConvertSidToUnicodeString(IN PUNICODE_STRING String,
+                             IN PSID Sid_,
+                             IN BOOLEAN AllocateBuffer)
 {
     WCHAR Buffer[256];
     PWSTR wcs;
     SIZE_T Length;
     ULONG i;
     PISID Sid = Sid_;
-
-    PAGED_CODE_RTL();
-
-    if (RtlValidSid (Sid) == FALSE)
-        return STATUS_INVALID_SID;
+    PAGED_CODE_RTL();
+
+    if (!RtlValidSid(Sid)) return STATUS_INVALID_SID;
 
     wcs = Buffer;
-    wcs += swprintf(wcs, L"S-%u-", Sid->Revision);
-
-    if (Sid->IdentifierAuthority.Value[0] == 0 &&
-        Sid->IdentifierAuthority.Value[1] == 0)
+    wcs += swprintf(wcs, L"S-1-");
+
+    if ((Sid->IdentifierAuthority.Value[0] == 0) &&
+        (Sid->IdentifierAuthority.Value[1] == 0))
     {
         wcs += swprintf(wcs,
                         L"%lu",
@@ -385,33 +379,27 @@
 
     for (i = 0; i < Sid->SubAuthorityCount; i++)
     {
-        wcs += swprintf(wcs,
-                        L"-%u",
-                        Sid->SubAuthority[i]);
+        wcs += swprintf(wcs, L"-%u", Sid->SubAuthority[i]);
     }
 
     if (AllocateBuffer)
     {
-        if (!RtlCreateUnicodeString(String,
-                                    Buffer))
+        if (!RtlCreateUnicodeString(String, Buffer)) return STATUS_NO_MEMORY;
+    }
+    else
+    {
+        Length = (wcs - Buffer) * sizeof(WCHAR);
+
+        if (Length > String->MaximumLength) return STATUS_BUFFER_TOO_SMALL;
+
+        String->Length = (USHORT)Length;
+        RtlCopyMemory(String->Buffer, Buffer, Length);
+
+        if (Length < String->MaximumLength)
         {
-            return STATUS_NO_MEMORY;
+            String->Buffer[Length / sizeof(WCHAR)] = UNICODE_NULL;
         }
     }
-    else
-    {
-        Length = (wcs - Buffer) * sizeof(WCHAR);
-
-        if (Length > String->MaximumLength)
-            return STATUS_BUFFER_TOO_SMALL;
-
-        String->Length = (USHORT)Length;
-        RtlCopyMemory(String->Buffer,
-                      Buffer,
-                      Length);
-        if (Length < String->MaximumLength)
-            String->Buffer[Length / sizeof(WCHAR)] = 0;
-    }
 
     return STATUS_SUCCESS;
 }