On Wed, Sep 04, 2013 at 09:30:37AM +0200, Ondrej Kos wrote:
> On 09/03/2013 12:08 AM, Jakub Hrozek wrote:
> >On Tue, Aug 27, 2013 at 12:19:39PM +0200, Ondrej Kos wrote:
> >>On 08/26/2013 03:53 PM, Jakub Hrozek wrote:
> >>>On Mon, Aug 26, 2013 at 02:58:18PM +0200, Ondrej Kos wrote:
> >>>>Hi,
> >>>>
> >>>>Attached patch adds sysdb routine to search users/groups by their
> >>>>SID, which will be needed for ticket 1568.
> >>>>
> >>>>I'm sending it now, because one of the patches I have in this
> >>>>working branch (store group SID) was already written and posted on
> >>>>the list by Sumit, so not to waste time again :)
> >>>>
> >>>
> >>>There is quite some code duplication between the two functions. Can we
> >>>have a single one that would also take a search base and either
> >>>objectlass or filter as arguments? The objectclass or filter would then
> >>>be and-end with SYSDB_SID_STR=%s. User and group functions could then be
> >>>just thin wrappers.
> >>>
> >>>Also I would prefer a unit test for any new sysdb API.
> >>>_______________________________________________
> >>>sssd-devel mailing list
> >>>sssd-devel@lists.fedorahosted.org
> >>>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >>>
> >>
> >>New patch attached.
> >
> >This is better, but do you need the generic function and the enum
> >exposed in the header? Can you make the generic function static and move
> >the enum inside the module?
> >
> >Also, instead of the enum, maybe the function can accept the format
> >strings directly and then we wouldn't need the adhoc enum at all.
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel@lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> New patch attached
> 
> Ondra

Only two more changes:

enum sysdb_sid_search_type can be removed, it's not used anywhere.

sysdb_search_entry_by_sid_str should report failure on a higher DEBUG
level. Feel free to also file a ticket or send a patch to fix the other
calls. I think in general OP_FAILURE should be much better.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to