r6071: * clean up UNISTR2_ARRAY ( really just an array of UNISTR4 + count )

* add some backwards compatibility to 'net rpc rights list'
* verify privilege name in 'net rpc rights privileges <name>' in order
  to give back better error messages.
(This used to be commit 0e29dc8aa384dfa6d2495beb8a9ffb5371e60a13)

10 files changed, 185 insertions, 131 deletions

diff --git a/source3/include/rpc_lsa.h b/source3/include/rpc_lsa.h
index 507161109f..69d8f53a6d 100644
--- a/source3/include/rpc_lsa.h
+++ b/source3/include/rpc_lsa.h
@@ -68,7 +68,7 @@
 #define LSA_LOOKUPPRIVNAME     0x20
 #define LSA_PRIV_GET_DISPNAME  0x21
 #define LSA_DELETEOBJECT       0x22
-#define LSA_ENUMACCTWITHRIGHT  0x23
+#define LSA_ENUMACCTWITHRIGHT  0x23	/* TODO: implement this one  -- jerry */
 #define LSA_ENUMACCTRIGHTS     0x24
 #define LSA_ADDACCTRIGHTS      0x25
 #define LSA_REMOVEACCTRIGHTS   0x26
@@ -532,7 +532,7 @@ typedef struct
 typedef struct
 {
 	uint32 count;
-	UNISTR2_ARRAY rights;
+	UNISTR4_ARRAY *rights;
 	NTSTATUS status;
 } LSA_R_ENUM_ACCT_RIGHTS;
 
@@ -542,8 +542,8 @@ typedef struct
 {
 	POLICY_HND pol; /* policy handle */
 	DOM_SID2 sid;
-	UNISTR2_ARRAY rights;
 	uint32 count;
+	UNISTR4_ARRAY *rights;
 } LSA_Q_ADD_ACCT_RIGHTS;
 
 /* LSA_R_ADD_ACCT_RIGHTS - LSA add account rights */
@@ -559,8 +559,8 @@ typedef struct
 	POLICY_HND pol; /* policy handle */
 	DOM_SID2 sid;
 	uint32 removeall;
-	UNISTR2_ARRAY rights;
 	uint32 count;
+	UNISTR4_ARRAY *rights;
 } LSA_Q_REMOVE_ACCT_RIGHTS;
 
 /* LSA_R_REMOVE_ACCT_RIGHTS - LSA remove account rights */
diff --git a/source3/include/rpc_misc.h b/source3/include/rpc_misc.h
index 16611fe955..d5dc35f6c7 100644
--- a/source3/include/rpc_misc.h
+++ b/source3/include/rpc_misc.h
@@ -182,13 +182,6 @@ typedef struct unistr2_info
 	uint16 *buffer;
 } UNISTR2;
 
-/* UNIHDR + UNISTR2* */
-typedef struct {
-	uint16 length;	/* number of bytes not counting NULL terminatation */
-	uint16 size;	/* number of bytes including NULL terminatation */
-	UNISTR2 *string;
-} UNISTR4;
-
 /* STRING2 - string size (in uint8 chars) and buffer */
 typedef struct string2_info
 {
@@ -206,22 +199,18 @@ typedef struct unistr3_info
 
 } UNISTR3;
 
-/* an element in a unicode string array */
-typedef struct
-{
-	uint16 length;
-	uint16 size;
-	uint32 ref_id;
-	UNISTR2 string;
-} UNISTR2_ARRAY_EL;
+/* UNIHDR + UNISTR2* */
 
-/* an array of unicode strings */
-typedef struct 
-{
-	uint32 ref_id;
+typedef struct {
+	uint16 length;	/* number of bytes not counting NULL terminatation */
+	uint16 size;	/* number of bytes including NULL terminatation */
+	UNISTR2 *string;
+} UNISTR4;
+
+typedef struct {
 	uint32 count;
-	UNISTR2_ARRAY_EL *strings;
-} UNISTR2_ARRAY;
+	UNISTR4 *strings;
+} UNISTR4_ARRAY;
 
 
 /* an element in a sid array */
diff --git a/source3/include/rpc_svcctl.h b/source3/include/rpc_svcctl.h
index fd24ec7ca9..90b90bd24b 100644
--- a/source3/include/rpc_svcctl.h
+++ b/source3/include/rpc_svcctl.h
@@ -166,7 +166,7 @@ typedef struct {
 typedef struct {
 	POLICY_HND handle;
 	uint32 parmcount;
-	UNISTR2_ARRAY parameters;
+	UNISTR4_ARRAY *parameters;
 } SVCCTL_Q_START_SERVICE;
 
 typedef struct {
diff --git a/source3/rpc_client/cli_lsarpc.c b/source3/rpc_client/cli_lsarpc.c
index 45b7509d45..81d535e54c 100644
--- a/source3/rpc_client/cli_lsarpc.c
+++ b/source3/rpc_client/cli_lsarpc.c
@@ -1260,12 +1260,16 @@ NTSTATUS cli_lsa_enum_account_rights(struct cli_state *cli, TALLOC_CTX *mem_ctx,
 	}
 
 	
-	privileges = TALLOC_ARRAY(mem_ctx, fstring, *count);
-	names = TALLOC_ARRAY(mem_ctx, char *, *count);
+	privileges = TALLOC_ARRAY( mem_ctx, fstring, *count );
+	names      = TALLOC_ARRAY( mem_ctx, char *, *count );
+
 	for ( i=0; i<*count; i++ ) {
-		/* ensure NULL termination ... what a hack */
-		pull_ucs2(NULL, privileges[i], r.rights.strings[i].string.buffer, 
-			sizeof(fstring), r.rights.strings[i].string.uni_str_len*2 , 0);
+		UNISTR4 *uni_string = &r.rights->strings[i];
+
+		if ( !uni_string->string )
+			continue;
+
+		rpcstr_pull( privileges[i], uni_string->string->buffer, sizeof(privileges[i]), -1, STR_TERMINATE );
 			
 		/* now copy to the return array */
 		names[i] = talloc_strdup( mem_ctx, privileges[i] );
@@ -1284,7 +1288,8 @@ done:
 
 NTSTATUS cli_lsa_add_account_rights(struct cli_state *cli, TALLOC_CTX *mem_ctx,
 				    POLICY_HND *pol, DOM_SID sid,
-				    uint32 count, const char **privs_name)
+				    
+uint32 count, const char **privs_name)
 {
 	prs_struct qbuf, rbuf;
 	LSA_Q_ADD_ACCT_RIGHTS q;
diff --git a/source3/rpc_client/cli_svcctl.c b/source3/rpc_client/cli_svcctl.c
index 19bf419983..9f80bb79a3 100644
--- a/source3/rpc_client/cli_svcctl.c
+++ b/source3/rpc_client/cli_svcctl.c
@@ -323,8 +323,8 @@ WERROR cli_svcctl_start_service( struct cli_state *cli, TALLOC_CTX *mem_ctx,
 	
 	memcpy( &in.handle, hService, sizeof(POLICY_HND) );
 	
-	in.parmcount = 0;
-	in.parameters.ref_id = 0x0;
+	in.parmcount  = 0;
+	in.parameters = NULL;
 	
 	CLI_DO_RPC( cli, mem_ctx, PI_SVCCTL, SVCCTL_START_SERVICE_W,
 	            in, out, 
diff --git a/source3/rpc_parse/parse_lsa.c b/source3/rpc_parse/parse_lsa.c
index d0b9b20a3b..649cb7845a 100644
--- a/source3/rpc_parse/parse_lsa.c
+++ b/source3/rpc_parse/parse_lsa.c
@@ -2322,7 +2322,9 @@ NTSTATUS init_r_enum_acct_rights( LSA_R_ENUM_ACCT_RIGHTS *r_u, PRIVILEGE_SET *pr
 	}
 
 	if ( num_priv ) {
-		if ( !init_unistr2_array( &r_u->rights, num_priv, privname_array ) ) 
+		r_u->rights = TALLOC_P( get_talloc_ctx(), UNISTR4_ARRAY );
+
+		if ( !init_unistr4_array( r_u->rights, num_priv, privname_array ) ) 
 			return NT_STATUS_NO_MEMORY;
 
 		r_u->count = num_priv;
@@ -2364,7 +2366,7 @@ BOOL lsa_io_r_enum_acct_rights(const char *desc, LSA_R_ENUM_ACCT_RIGHTS *r_c, pr
 	if(!prs_uint32("count   ", ps, depth, &r_c->count))
 		return False;
 
-	if(!smb_io_unistr2_array("rights", &r_c->rights, ps, depth))
+	if ( !prs_pointer("rights", ps, depth, (void**)&r_c->rights, sizeof(UNISTR4_ARRAY), (PRS_POINTER_CAST)prs_unistr4_array) )
 		return False;
 
 	if(!prs_align(ps))
@@ -2380,17 +2382,17 @@ BOOL lsa_io_r_enum_acct_rights(const char *desc, LSA_R_ENUM_ACCT_RIGHTS *r_c, pr
 /*******************************************************************
  Inits an LSA_Q_ADD_ACCT_RIGHTS structure.
 ********************************************************************/
-void init_q_add_acct_rights(LSA_Q_ADD_ACCT_RIGHTS *q_q, 
-			    POLICY_HND *hnd, 
-			    DOM_SID *sid,
-			    uint32 count, 
-			    const char **rights)
+void init_q_add_acct_rights( LSA_Q_ADD_ACCT_RIGHTS *q_q, POLICY_HND *hnd, 
+                             DOM_SID *sid, uint32 count, const char **rights )
 {
 	DEBUG(5, ("init_q_add_acct_rights\n"));
 
 	q_q->pol = *hnd;
 	init_dom_sid2(&q_q->sid, sid);
-	init_unistr2_array(&q_q->rights, count, rights);
+	
+	q_q->rights = TALLOC_P( get_talloc_ctx(), UNISTR4_ARRAY );
+	init_unistr4_array( q_q->rights, count, rights );
+	
 	q_q->count = count;
 }
 
@@ -2412,7 +2414,7 @@ BOOL lsa_io_q_add_acct_rights(const char *desc, LSA_Q_ADD_ACCT_RIGHTS *q_q, prs_
 	if(!prs_uint32("count", ps, depth, &q_q->count))
 		return False;
 
-	if(!smb_io_unistr2_array("rights", &q_q->rights, ps, depth))
+	if ( !prs_pointer("rights", ps, depth, (void**)&q_q->rights, sizeof(UNISTR4_ARRAY), (PRS_POINTER_CAST)prs_unistr4_array) )
 		return False;
 
 	return True;
@@ -2446,10 +2448,14 @@ void init_q_remove_acct_rights(LSA_Q_REMOVE_ACCT_RIGHTS *q_q,
 	DEBUG(5, ("init_q_remove_acct_rights\n"));
 
 	q_q->pol = *hnd;
+
 	init_dom_sid2(&q_q->sid, sid);
+
 	q_q->removeall = removeall;
-	init_unistr2_array(&q_q->rights, count, rights);
 	q_q->count = count;
+
+	q_q->rights = TALLOC_P( get_talloc_ctx(), UNISTR4_ARRAY );
+	init_unistr4_array( q_q->rights, count, rights );
 }
 
 
@@ -2473,7 +2479,7 @@ BOOL lsa_io_q_remove_acct_rights(const char *desc, LSA_Q_REMOVE_ACCT_RIGHTS *q_q
 	if(!prs_uint32("count", ps, depth, &q_q->count))
 		return False;
 
-	if(!smb_io_unistr2_array("rights", &q_q->rights, ps, depth))
+	if ( !prs_pointer("rights", ps, depth, (void**)&q_q->rights, sizeof(UNISTR4_ARRAY), (PRS_POINTER_CAST)prs_unistr4_array) )
 		return False;
 
 	return True;
diff --git a/source3/rpc_parse/parse_misc.c b/source3/rpc_parse/parse_misc.c
index 76c6438d59..57f44f9f85 100644
--- a/source3/rpc_parse/parse_misc.c
+++ b/source3/rpc_parse/parse_misc.c
@@ -1114,7 +1114,6 @@ BOOL smb_io_unistr2(const char *desc, UNISTR2 *uni2, uint32 buffer, prs_struct *
 
 BOOL prs_unistr4(const char *desc, prs_struct *ps, int depth, UNISTR4 *uni4)
 {
-
 	if ( !prs_uint16("length", ps, depth, &uni4->length ))
 		return False;
 	if ( !prs_uint16("size", ps, depth, &uni4->size ))
@@ -1126,33 +1125,97 @@ BOOL prs_unistr4(const char *desc, prs_struct *ps, int depth, UNISTR4 *uni4)
 	return True;
 }
 
+/*******************************************************************
+ now read/write UNISTR4 header
+********************************************************************/
+
+BOOL prs_unistr4_hdr(const char *desc, prs_struct *ps, int depth, UNISTR4 *uni4)
+{
+	prs_debug(ps, depth, desc, "prs_unistr4_hdr");
+	depth++;
+
+	if ( !prs_uint16("length", ps, depth, &uni4->length) )
+		return False;
+	if ( !prs_uint16("size", ps, depth, &uni4->size) )
+		return False;
+	if ( !prs_io_unistr2_p(desc, ps, depth, &uni4->string) )
+		return False;
+		
+	return True;
+}
+
+/*******************************************************************
+ now read/write UNISTR4 string
+********************************************************************/
+
+BOOL prs_unistr4_str(const char *desc, prs_struct *ps, int depth, UNISTR4 *uni4)
+{
+	prs_debug(ps, depth, desc, "prs_unistr4_str");
+	depth++;
+
+	if ( !prs_io_unistr2(desc, ps, depth, uni4->string) )
+		return False;
+		
+	return True;
+}
+
+/*******************************************************************
+ Reads or writes a UNISTR2_ARRAY structure.
+********************************************************************/
+
+BOOL prs_unistr4_array(const char *desc, prs_struct *ps, int depth, UNISTR4_ARRAY *array )
+{
+	unsigned int i;
+
+	prs_debug(ps, depth, desc, "prs_unistr4_array");
+	depth++;
+
+	if(!prs_uint32("count", ps, depth, &array->count))
+		return False;
+
+	if ( array->count == 0 ) 
+		return True;
+	
+	if (UNMARSHALLING(ps)) {
+		if ( !(array->strings = TALLOC_ZERO_ARRAY( get_talloc_ctx(), UNISTR4, array->count)) )
+			return False;
+	}
+	
+	/* write the headers and then the actual string buffer */
+	
+	for ( i=0; i<array->count; i++ ) {
+		if ( !prs_unistr4_hdr( "string", ps, depth, &array->strings[i]) )
+			return False;
+	}
+
+	for (i=0;i<array->count;i++) {
+		if ( !prs_unistr4_str("string", ps, depth, &array->strings[i]) ) 
+			return False;
+	}
+	
+	return True;
+}
 
 /********************************************************************
   initialise a UNISTR_ARRAY from a char**
 ********************************************************************/
 
-BOOL init_unistr2_array(UNISTR2_ARRAY *array, 
-		       uint32 count, const char **strings)
+BOOL init_unistr4_array( UNISTR4_ARRAY *array, uint32 count, const char **strings )
 {
 	unsigned int i;
 
 	array->count = count;
-	array->ref_id = count?1:0;
-	if (array->count == 0) {
+
+	if ( array->count == 0 )
 		return True;
-	}
 
-	array->strings = TALLOC_ZERO_ARRAY(get_talloc_ctx(), UNISTR2_ARRAY_EL, count );
-	if (!array->strings) {
+	/* allocate memory for the array of UNISTR4 objects */
+
+	if ( !(array->strings = TALLOC_ZERO_ARRAY(get_talloc_ctx(), UNISTR4, count )) )
 		return False;
-	}
 
-	for (i=0;i<count;i++) {
-		init_unistr2(&array->strings[i].string, strings[i], UNI_FLAGS_NONE);
-		array->strings[i].size = array->strings[i].string.uni_max_len*2;
-		array->strings[i].length = array->strings[i].size;
-		array->strings[i].ref_id = 1;
-	}
+	for ( i=0; i<count; i++ ) 
+		init_unistr4( &array->strings[i], strings[i], STR_TERMINATE );
 
 	return True;
 }
@@ -1205,55 +1268,6 @@ BOOL smb_io_account_lockout_str(const char *desc, LOCKOUT_STRING *account_lockou
 }
 
 /*******************************************************************
- Reads or writes a UNISTR2_ARRAY structure.
-********************************************************************/
-BOOL smb_io_unistr2_array(const char *desc, UNISTR2_ARRAY *array, prs_struct *ps, int depth)
-{
-	unsigned int i;
-
-	prs_debug(ps, depth, desc, "smb_io_unistr2_array");
-	depth++;
-
-	if(!prs_uint32("ref_id", ps, depth, &array->ref_id))
-		return False;
-
-	if (! array->ref_id) {
-		return True;
-	}
-
-	if(!prs_uint32("count", ps, depth, &array->count))
-		return False;
-
-	if (array->count == 0) {
-		return True;
-	}
-
-	if (UNMARSHALLING(ps)) {
-		array->strings = TALLOC_ZERO_ARRAY(get_talloc_ctx(), UNISTR2_ARRAY_EL, array->count );
-	}
-	if (! array->strings) {
-		return False;
-	}
-
-	for (i=0;i<array->count;i++) {
-		if(!prs_uint16("length", ps, depth, &array->strings[i].length))
-			return False;
-		if(!prs_uint16("size", ps, depth, &array->strings[i].size))
-			return False;
-		if(!prs_uint32("ref_id", ps, depth, &array->strings[i].ref_id))
-			return False;
-	}
-
-	for (i=0;i<array->count;i++) {
-		if (! smb_io_unistr2("string", &array->strings[i].string, array->strings[i].ref_id, ps, depth)) 
-			return False;
-	}
-	
-	return True;
-}
-
-
-/*******************************************************************
  Inits a DOM_RID2 structure.
 ********************************************************************/
 
diff --git a/source3/rpc_parse/parse_svcctl.c b/source3/rpc_parse/parse_svcctl.c
index acebcf34c6..1c41a18b99 100644
--- a/source3/rpc_parse/parse_svcctl.c
+++ b/source3/rpc_parse/parse_svcctl.c
@@ -481,7 +481,7 @@ BOOL svcctl_io_q_start_service(const char *desc, SVCCTL_Q_START_SERVICE *q_u, pr
 	if(!prs_uint32("parmcount", ps, depth, &q_u->parmcount))
 		return False;
 
-	if(!smb_io_unistr2_array("parameters", &q_u->parameters, ps, depth))
+	if ( !prs_pointer("rights", ps, depth, (void**)&q_u->parameters, sizeof(UNISTR4_ARRAY), (PRS_POINTER_CAST)prs_unistr4_array) )
 		return False;
 
 	return True;
diff --git a/source3/rpc_server/srv_lsa_nt.c b/source3/rpc_server/srv_lsa_nt.c
index 7ea35a91fa..db1aa57ea9 100644
--- a/source3/rpc_server/srv_lsa_nt.c
+++ b/source3/rpc_server/srv_lsa_nt.c
@@ -1337,7 +1337,7 @@ NTSTATUS _lsa_add_acct_rights(pipes_struct *p, LSA_Q_ADD_ACCT_RIGHTS *q_u, LSA_R
 	int i = 0;
 	DOM_SID sid;
 	fstring privname;
-	UNISTR2_ARRAY *uni_privnames = &q_u->rights;
+	UNISTR4_ARRAY *uni_privnames = q_u->rights;
 	struct current_user user;
 	
 
@@ -1368,11 +1368,16 @@ NTSTATUS _lsa_add_acct_rights(pipes_struct *p, LSA_Q_ADD_ACCT_RIGHTS *q_u, LSA_R
 	}
 		
 	for ( i=0; i<q_u->count; i++ ) {
-		unistr2_to_ascii( privname, &uni_privnames->strings[i].string, sizeof(fstring)-1 );
-		
+		UNISTR4 *uni4_str = &uni_privnames->strings[i];
+
 		/* only try to add non-null strings */
+
+		if ( !uni4_str->string )
+			continue;
+
+		rpcstr_pull( privname, uni4_str->string->buffer, sizeof(privname), -1, STR_TERMINATE );
 		
-		if ( *privname && !grant_privilege_by_name( &sid, privname ) ) {
+		if ( !grant_privilege_by_name( &sid, privname ) ) {
 			DEBUG(2,("_lsa_add_acct_rights: Failed to add privilege [%s]\n", privname ));
 			return NT_STATUS_NO_SUCH_PRIVILEGE;
 		}
@@ -1390,7 +1395,7 @@ NTSTATUS _lsa_remove_acct_rights(pipes_struct *p, LSA_Q_REMOVE_ACCT_RIGHTS *q_u,
 	int i = 0;
 	DOM_SID sid;
 	fstring privname;
-	UNISTR2_ARRAY *uni_privnames = &q_u->rights;
+	UNISTR4_ARRAY *uni_privnames = q_u->rights;
 	struct current_user user;
 	
 
@@ -1425,11 +1430,16 @@ NTSTATUS _lsa_remove_acct_rights(pipes_struct *p, LSA_Q_REMOVE_ACCT_RIGHTS *q_u,
 	}
 		
 	for ( i=0; i<q_u->count; i++ ) {
-		unistr2_to_ascii( privname, &uni_privnames->strings[i].string, sizeof(fstring)-1 );
-		
+		UNISTR4 *uni4_str = &uni_privnames->strings[i];
+
 		/* only try to add non-null strings */
+
+		if ( !uni4_str->string )
+			continue;
+
+		rpcstr_pull( privname, uni4_str->string->buffer, sizeof(privname), -1, STR_TERMINATE );
 		
-		if ( *privname && !revoke_privilege_by_name( &sid, privname ) ) {
+		if ( !revoke_privilege_by_name( &sid, privname ) ) {
 			DEBUG(2,("_lsa_remove_acct_rights: Failed to revoke privilege [%s]\n", privname ));
 			return NT_STATUS_NO_SUCH_PRIVILEGE;
 		}
@@ -1439,6 +1449,9 @@ NTSTATUS _lsa_remove_acct_rights(pipes_struct *p, LSA_Q_REMOVE_ACCT_RIGHTS *q_u,
 }
 
 
+/***************************************************************************
+ ***************************************************************************/
+
 NTSTATUS _lsa_enum_acct_rights(pipes_struct *p, LSA_Q_ENUM_ACCT_RIGHTS *q_u, LSA_R_ENUM_ACCT_RIGHTS *r_u)
 {
 	struct lsa_info *info = NULL;
@@ -1478,6 +1491,9 @@ NTSTATUS _lsa_enum_acct_rights(pipes_struct *p, LSA_Q_ENUM_ACCT_RIGHTS *q_u, LSA
 }
 
 
+/***************************************************************************
+ ***************************************************************************/
+
 NTSTATUS _lsa_lookup_priv_value(pipes_struct *p, LSA_Q_LOOKUP_PRIV_VALUE *q_u, LSA_R_LOOKUP_PRIV_VALUE *r_u)
 {
 	struct lsa_info *info = NULL;
diff --git a/source3/utils/net_rpc_rights.c b/source3/utils/net_rpc_rights.c
index ce95226951..3a986ed251 100644
--- a/source3/utils/net_rpc_rights.c
+++ b/source3/utils/net_rpc_rights.c
@@ -284,16 +284,23 @@ static NTSTATUS rpc_rights_list_internal( const DOM_SID *domain_sid, const char
 	POLICY_HND pol;
 	NTSTATUS result;
 	DOM_SID sid;
+	fstring privname;
+	fstring description;
+	uint16 lang_id = 0;
+	uint16 lang_id_sys = 0;
+	uint16 lang_id_desc;
+	
 	
 	result = cli_lsa_open_policy(cli, mem_ctx, True, 
 		SEC_RIGHTS_MAXIMUM_ALLOWED, &pol);
 
 	if ( !NT_STATUS_IS_OK(result) )
 		return result;
-		
+	
+	/* backwards compatibility; just list available privileges if no arguement */
+	   
 	if (argc == 0) {
-		d_printf("Usage: net rpc rights list [accounts|privileges] [name|SID]\n");
-		result = NT_STATUS_OK;
+		result = enum_privileges( mem_ctx, cli, &pol );
 		goto done;
 	}
 
@@ -305,18 +312,35 @@ static NTSTATUS rpc_rights_list_internal( const DOM_SID *domain_sid, const char
 			goto done;
 		}
 
-		while (argv[i] != NULL) {
-			result = enum_accounts_for_privilege(mem_ctx, cli, &pol, argv[i]);
+		while ( argv[i] != NULL ) 
+		{
+			fstrcpy( privname, argv[i] );
+			i++;
+		
+			/* verify that this is a valid privilege for error reporting */
+			
+			result = cli_lsa_get_dispname(cli, mem_ctx, &pol, privname, lang_id, 
+				lang_id_sys, description, &lang_id_desc);
+			
+			if ( !NT_STATUS_IS_OK(result) ) {
+				if ( NT_STATUS_EQUAL( result, NT_STATUS_NO_SUCH_PRIVILEGE ) ) 
+					d_printf("No such privilege exists: %s.\n", privname);
+				else
+					d_printf("Error resolving privilege display name [%s].\n", nt_errstr(result));
+				continue;
+			}
+			
+			result = enum_accounts_for_privilege(mem_ctx, cli, &pol, privname);
 			if (!NT_STATUS_IS_OK(result)) {
-				goto done;
+				d_printf("Error enumerating accounts for privilege %s [%s].\n", 
+					privname, nt_errstr(result));
+				continue;
 			}
-			i++;
 		}
 		goto done;
 	}
 
-	/* special case to enuemrate all privileged SIDs 
-	   with associated rights */
+	/* special case to enumerate all privileged SIDs with associated rights */
 	
 	if (strequal( argv[0], "accounts")) {
 		int i = 1;
@@ -343,7 +367,7 @@ static NTSTATUS rpc_rights_list_internal( const DOM_SID *domain_sid, const char
 	/* backward comaptibility: if no keyword provided, treat the key
 	   as an account name */
 	if (argc > 1) {
-		d_printf("Usage: net rpc rights list [accounts|privileges] [name|SID]\n");
+		d_printf("Usage: net rpc rights list [[accounts|privileges] [name|SID]]\n");
 		result = NT_STATUS_OK;
 		goto done;
 	}
@@ -487,9 +511,9 @@ static int rpc_rights_revoke( int argc, const char **argv )
 
 static int net_help_rights( int argc, const char **argv )
 {
-	d_printf("net rpc rights list [accounts|username]   View available or assigned privileges\n");
-	d_printf("net rpc rights grant <name|SID> <right>   Assign privilege[s]\n");
-	d_printf("net rpc rights revoke <name|SID> <right>  Revoke privilege[s]\n");
+	d_printf("net rpc rights list [{accounts|privileges} [name|SID]]   View available or assigned privileges\n");
+	d_printf("net rpc rights grant <name|SID> <right>                  Assign privilege[s]\n");
+	d_printf("net rpc rights revoke <name|SID> <right>                 Revoke privilege[s]\n");
 	
 	d_printf("\nBoth 'grant' and 'revoke' require a SID and a list of privilege names.\n");
 	d_printf("For example\n");