Stephen Gallagher <sgallagh(a)redhat.com> wrote:
On 09/16/2010 05:05 PM, Stephen Gallagher wrote:
> I've rewritten these patches. Now, instead of searching for individual
> netgroup entries, the code will take advantage of the memberOf plugin to
> return all netgroup triples in a single call to sysdb_getnetgr()
>
> This approach will make life easier with LDAP as well, as we will have
> built-in loop prevention.
>
> Please review these patches while my netgroup work is ongoing.
>
> Patch 0001: New sysdb interfaces for netgroups. Add and delete support
> for netgroups entries, tuples and nested netgroups.
>
> Patch 0002: Unit tests for the above interfaces.
Yet another change to these patches.
I realized that I actually need to get back the three strings from the
triple instead of the internal representation. It makes the most sense
for this to be done in sysdb_getnetgr(), so attached are two new patches
which changes the interface (again) to return a structure of three
strings instead of a single string representing the tuple.
I went through the code of the first patch and there are some things which
don't seem to be right.
First of all, the code is missing the memory hierarchy entirely. Here are some
examples:
src/db/sys_ops.c:
1125, 1175, 2504, 2567 - parent context should be taken as the first argument
1132, 1186, 2517, 2578 - parent context should be here instead of NULL
Functions sysdb_getnetgr and sysdb_netgr_split_triple in src/db/sysdb_search.c
also allocate tmp context outside the hierarchy, I hope I didn't forget any
other occurence.
I'm aware that these aren't causing any error right now, but for the future it
is safe to allocate the memory strictly in the correct parent context.
Regarding errno_t: you use it in several functions. That's probably right, but
I think if we want to take this approach, we should update all functions to
follow rule. For example function sysdb_add_netgroup should use it for
returning errno as well.
Then I suggest a minor improvement:
src/db/sys_ops.c:1230 - the DEBUG macro will be better if called just before
the talloc_zfree on 1233 (in case committing the transaction failed)
I hope I have all that line numbers correct.
--
Jan