On Fri, Jun 28, 2013 at 09:53:42PM +0200, Jakub Hrozek wrote:
> On Fri, Jun 28, 2013 at 09:37:44PM +0200, Sumit Bose wrote:
> > On Fri, Jun 28, 2013 at 09:23:27PM +0200, Jakub Hrozek wrote:
> > > On Fri, Jun 28, 2013 at 08:59:38PM +0200, Sumit Bose wrote:
> > > > On Fri, Jun 28, 2013 at 06:43:48PM +0200, Jakub Hrozek wrote:
> > > > > Hi,
> > > > >
> > > > > the attached patches implement
> > > > >
https://fedorahosted.org/sssd/ticket/1962. When a new option,
> > > > > ipa_server_mode is set to True, then subdomain/trusted users are
not
> > > > > looked up using the extop plugin but AD ID context is
initialized and
> > > > > the users are looked up directly with AD code. This is in
support of
> > > > > legacy clients looking up trusted AD users and groups.
> > > >
> > > > Patches are working well, now trusted domain users and groups can
have
> > > > algorithmically mapped IDs or IDs from POSIX attributes from the
trusted
> > > > DC.
> > > >
> > > > [PATCH 1/8] IPA: Add a server mode option
> > > > ACK
> > > >
> > > > [PATCH 2/8] LDAP: Add utility function sdap_copy_map
> > > > >
> > > > > +
> > > > > + for (i = 0; i < num_entries; i++) {
> > > > > + map[i].opt_name = src_map[i].opt_name;
> > > > > + map[i].def_name = src_map[i].def_name;
> > > > > + map[i].name = discard_const(src_map[i].def_name);
> > > > > + map[i].sys_name = src_map[i].sys_name;
> > > > > +
> > > >
> > >
> > > Yes, it would. I have changed the patch. I was just trying to avoid
> > > doing more mallocs than necessary, but actually that already burned me
> > > once with the code as some other part of the back end assumes that the
> > > map is talloc context.
> >
> > >
> > > Subject: [PATCH 2/8] LDAP: Add utility function sdap_copy_map
> > >
> > > The AD subdomains will only use default options values. This patch
> > > introduces a new utility function sdap_copy_map() that copies the
> > > default options map.
> > >
> > > Subtask of:
> > >
https://fedorahosted.org/sssd/ticket/1962
> > > ---
> > > src/providers/ldap/sdap.c | 33 +++++++++++++++++++++++++++++++++
> > > src/providers/ldap/sdap.h | 5 +++++
> > > 2 files changed, 38 insertions(+)
> > >
> > > diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
> > > index
0492be05d78b78967989ecd783d92ce94315c551..3765a2a24885b348dd0b5f515e46d5858676bf98 100644
> > > --- a/src/providers/ldap/sdap.c
> > > +++ b/src/providers/ldap/sdap.c
> > > @@ -28,6 +28,39 @@
> > >
> > > /*
=Retrieve-Options====================================================== */
> > >
> > > +int sdap_copy_map(TALLOC_CTX *memctx,
> > > + struct sdap_attr_map *src_map,
> > > + int num_entries,
> > > + struct sdap_attr_map **_map)
> > > +{
> > > + struct sdap_attr_map *map;
> > > + int i;
> > > +
> > > + map = talloc_array(memctx, struct sdap_attr_map, num_entries);
> > > + if (!map) {
> > > + return ENOMEM;
> > > + }
> > > +
> > > + for (i = 0; i < num_entries; i++) {
> > > + map[i].opt_name = talloc_strdup(map, src_map[i].opt_name);
> > > + map[i].def_name = talloc_strdup(map, src_map[i].def_name);
> > > + map[i].name = talloc_strdup(map, src_map[i].def_name);
> > > + map[i].sys_name = talloc_strdup(map, src_map[i].sys_name);
> > > +
> >
> > I think you should allow def_name and name to be NULL. I think if
> > opt_name or sys_name are NULL it's an error.
>
> Yes, you're right. New round of patches attached.
ACK to all.
bye,
Sumit