libcli/security:sddl_decode_err_msg(): don't pretend msg is optional (CID1548624)

Having it optionally NULL just complicates the code, and Coverity
rightly complained.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index 97e579cfe3290175d8467aa07b0426f3aea06ee1..f0f25b89b59f65418fb3b64e55edba120aef490b 100644 (file)
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -842,23 +842,23 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
                .forest_sid = domain_sid,
        };
        const char *start = sddl;
-       struct security_descriptor *sd;
+       struct security_descriptor *sd = NULL;
+
+       if (msg == NULL || msg_offset == NULL) {
+               DBG_ERR("Programmer misbehaviour: use sddl_decode() "
+                       "or provide msg pointers.\n");
+               return NULL;
+       }
+       *msg = NULL;
+       *msg_offset = 0;
+
        sd = talloc_zero(mem_ctx, struct security_descriptor);
        if (sd == NULL) {
-               goto failed;
+               return NULL;
        }
        sd->revision = SECURITY_DESCRIPTOR_REVISION_1;
        sd->type     = SEC_DESC_SELF_RELATIVE;
 
-       if (msg != NULL) {
-               if (msg_offset == NULL) {
-                       DBG_ERR("Programmer misbehaviour\n");
-                       goto failed;
-               }
-               *msg = NULL;
-               *msg_offset = 0;
-       }
-
        while (*sddl) {
                uint32_t flags;
                char c = sddl[0];
@@ -896,16 +896,14 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
        }
        return sd;
 failed:
-       if (msg != NULL) {
-               if (*msg != NULL) {
-                       *msg = talloc_steal(mem_ctx, *msg);
-               }
-               /*
-                * The actual message (*msg) might still be NULL, but the
-                * offset at least provides a clue.
-                */
-               *msg_offset += sddl - start;
+       if (*msg != NULL) {
+               *msg = talloc_steal(mem_ctx, *msg);
        }
+       /*
+        * The actual message (*msg) might still be NULL, but the
+        * offset at least provides a clue.
+        */
+       *msg_offset += sddl - start;
        DEBUG(2,("Badly formatted SDDL '%s'\n", sddl));
        talloc_free(sd);
        return NULL;