Recent changes to autorid (was Re: [SCM] Samba Shared Repository - branch master updated)

[Michael Adam]

Hi Simo and Christian,

On 2014-04-27 at 22:02 +0200, Christian Ambach wrote:
> Am 25.04.14 20:36, schrieb Simo:
> >On Fri, 2014-04-25 at 17:53 +0200, Michael Adam wrote:
> >>
> >>+#define IDMAP_AUTORID_ALLOC_RESERVED 500
> >>+
> >
> >Sorry to sound negative,

That's perfectly fine.

> >but I think this fiddling and making
> >strange exceptions with autorid is a bad idea.
>
> I somehow agree with Simo.

I feel bad about not having discussed this on the list
prior to pushing. (As you see, this has simmered for
a while and Metze has reviewed it.) I should have
brought it up for discussion, but in the end I was in
a hurry, and haste is a bad advisor...

I had some good reasons for doing these changes
and doing them this way, but on the other hand I
certainly understand the bad feelings about it.

I will write a detailed explanation tomorrow
(it is too late now). But I certainly offer to
revert the changes (that is the two patches that
add the special treatment for wellknowns) for now
if that is desired. (I think the other patches
are reasonable independent improvements of the code.)

Cheers - Michael

> Adding more and more exceptions and special
> handlings to autorid throws away the originally simple idea behind it
> and creates a more and more complex module. You now need to look at many
> code paths to understand how the different types of SIDs are treated and
> most of them miss the explanatory comments why some SIDs are treated
> differently. We definitely need more documentation here: both in the
> code and in the manpage.
> What is the rationale behind this change? Making sure all current
> well-knowns are place right next to each other and when Microsoft adds
> new well-knowns in the future they also be placed beside the existing
> ones? How can you make sure that 500 will be enough?
> 
> Looking at the changes, I corrected a typo in a new DEBUG message,
> please see attachment.
> 
> Cheers,
> Christian
> 

> From 6c81cbfe73bb6901611540cc22eb0a6da70b2034 Mon Sep 17 00:00:00 2001
> From: Christian Ambach <ambi at samba.org>
> Date: Sun, 27 Apr 2014 21:54:40 +0200
> Subject: [PATCH] autorid: fix a typo
> 
> Signed-off-by: Christian Ambach <ambi at samba.org>
> ---
>  source3/winbindd/idmap_autorid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
> index 37612c2..2cafcb5 100644
> --- a/source3/winbindd/idmap_autorid.c
> +++ b/source3/winbindd/idmap_autorid.c
> @@ -434,7 +434,7 @@ static NTSTATUS idmap_autorid_sid_to_id_alloc_action(
>  			return ret;
>  		}
>  
> -		DEBUG(10, ("Sepecial sid %s not mapped. falling back to "
> +		DEBUG(10, ("Special sid %s not mapped. falling back to "
>  			   "regular allocation\n",
>  			   sid_string_dbg(ctx->map->sid)));
>  	}
> -- 
> 1.8.3.2
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20140428/2e29644a/attachment.pgp>