On Mon, 2012-01-23 at 15:36 +0100, Jakub Hrozek wrote:
On Sun, Jan 22, 2012 at 08:24:42PM -0500, Stephen Gallagher wrote:
> On Sun, 2012-01-22 at 22:14 +0100, Jakub Hrozek wrote:
> > The attached patch requires the DP refactoring patch. It contains a
> > sudo-specific DP request.
> >
> > A patch that actually uses the request in the sudo responder is being
> > tested now, but I wanted to get an early response on the approach.
>
>
> Nack.
>
> Based on my review of the sss_dp_issue_request() patch, this will need
> updating to account for the hash key changes.
>
The key is now a string local to the _send() function, hash_key is now
created in sss_dp_issue_request()
> I'd rather see us start separating the different responder DP processing
> into their own source files. I think this belongs in responder/sudo/. We
> should reserve common for truly independent features (like the
> sss_dp_issue_request()).
OK, done. This patch puts the new request into sudosrv_dp.c and a later
one that actually uses this new request is going to remove the old
implementation.
Would you like to move the _account_send() request as well? I think
that "getting user info" is quite common among all the responders, so
the account request can stay in responders/common/ rather than
responders/nss.
>
> Also, when you call sss_dp_issue_request(), you're passing 'state' as
> the pvt data for sss_dp_get_sudoers_msg(), but you meant to pass 'info',
> I think.
Right, I had this fixed but squashed into another patch (otherwise I
wouldn't be able to test the new code). Sorry, it's fixed in the patch
now.
>
> Otherwise, this patch looks straightforward.
Thank you, a new patch is attached.
Ack (pending push of the dependent patches)