lib/param: fix usage of 'write list = +Group'

[Stefan (metze) Metzmacher]

Hi Andrew,

>> - Log -----------------------------------------------------------------
>> commit f83521a4ca26c8b14af1c407c49426d8dd8179f7
>> Author: Stefan Metzmacher <metze at samba.org>
>> Date:   Thu Aug 23 15:32:05 2012 +0200
>>
>>     lib/param: fix usage of 'write list = +Group'
>>     
>>     metze
>>     
>>     Autobuild-User(master): Björn Jacke <bj at sernet.de>
>>     Autobuild-Date(master): Fri Aug 24 11:28:17 CEST 2012 on sn-devel-104
>>
>> -----------------------------------------------------------------------
>>
>> Summary of changes:
>>  lib/param/loadparm.c |   10 ++++++++--
>>  1 files changed, 8 insertions(+), 2 deletions(-)
>>
>>
>> Changeset truncated at 500 lines:
>>
>> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
>> index 4751a06..3814087 100644
>> --- a/lib/param/loadparm.c
>> +++ b/lib/param/loadparm.c
>> @@ -1355,13 +1355,19 @@ static bool set_variable(TALLOC_CTX *mem_ctx, int parmnum, void *parm_ptr,
>>  			char **new_list = str_list_make(mem_ctx,
>>  							pszParmValue, NULL);
>>  			for (i=0; new_list[i]; i++) {
>> -				if (new_list[i][0] == '+' && new_list[i][1]) {
>> +				if (*(const char ***)parm_ptr != NULL &&
>> +				    new_list[i][0] == '+' &&
>> +				    new_list[i][1])
>> +				{
>>  					if (!str_list_check(*(const char ***)parm_ptr,
>>  							    &new_list[i][1])) {
>>  						*(const char ***)parm_ptr = str_list_add(*(const char ***)parm_ptr,
>>  											 &new_list[i][1]);
>>  					}
>> -				} else if (new_list[i][0] == '-' && new_list[i][1]) {
>> +				} else if (*(const char ***)parm_ptr != NULL &&
>> +					   new_list[i][0] == '-' &&
>> +					   new_list[i][1])
>> +				{
>>  					str_list_remove(*(const char ***)parm_ptr,
>>  							&new_list[i][1]);
>>  				} else {
> 
> This doesn't look quite right, because in other lists (such as server
> services) adding "+smb" to the empty list should give us "smb".

Does that ever happen, in that case we don't have an empty list
typically.

> What I think we need to do is separate lists further into:
>  - USER_LIST
>  - CMD_LIST
>  - LIST
> 
> CMD_LIST and USER_LIST would be very similar, but would separate tokens
> with a comma ',' and both would not have the magic +/- behaviour. 

I think we should give a special meaning to the +/- magic
and keep LIST simple. (But at least everything seems to work in the
current code)

BTW: I first thought of such an approach, but gladly found such a simple
patch
to fix the segfault.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120825/5097cf5c/attachment.pgp>