[Freeipa-devel] [PATCH][SSSD] Implement GetUserAttributes in the InfoPipe

[Simo Sorce]

Stephen,
I think there is some work to do on the patch, as it is it will not work
as, for example, the latest patches removed check_provider.
(It's always true except for LOCAL, so I removed it in favor of checking
explicitly for the domain name != LOCAL)

See some minor comments inline, but the general approach looks ok.

On Sun, 2009-03-01 at 15:19 -0500, Stephen Gallagher wrote:
> This patch adds support for requesting user data in the sysdb via the
> InfoPipe. It currently has support for reading defined entries of
> integral, floating-point or string types.

Some comments:

- When you use btreemap_get_value() like in infp_get_domain_obj() you
should probably use talloc_get_type() instead of doing a cast yourself,
so that if something wrong comes out of it you get a null pointer and do
not act on random data.

- why in create_getattr_result_map() do you fetch uint64_t data but
variant->data is cast to int ?

- In infp_get_all_attributes() why do you copy all these static strings?
As far as I can see they are not manipulated in any way, but just used
as is.

- I see that you added confdb/confbd.h to sysdb.h, but you also keep
including them both explicitly in infopipe, any reason for this ?

> Tasks remaining:
>     1) Implement call to the provider when cache is out of date
>     2) Support byte arrays for userpic and similar
> 
> I modified init_src_context in sysdb_search.c to accept an array of
> attributes to pass into the LDB search.

I would rather not make attrs a parameter of init_src_context() but just
assign it when necessary like we do for expression, given most of the
time we would pass in NULL otherwise.

> I also made one additional related fix: the btreemap now sorts in the
> correct order. Previously I had accidentally transposed the two values
> for sorting, so the map would always have been in exact reverse order.

Thanks for this.


Simo.

-- 
Simo Sorce * Red Hat, Inc * New York