On 11/12/2013 12:19 PM, Jakub Hrozek wrote:
On Tue, Nov 05, 2013 at 02:32:29PM +0100, Pavel Březina wrote:
> On 10/01/2013 09:03 PM, Jakub Hrozek wrote:
>> Hi,
>>
>> the attached two patches fix #2077. I wrote the patches even though
>> there are unresolved 1.11 issues because there was a customer looking
>> for a test package.
>
> Patch 1: Ack.
>
> Patch 2: Nack.
>
>> static struct tevent_req *get_user_dn_send(TALLOC_CTX *memctx,
>> struct tevent_context *ev,
>> struct sss_domain_info *domain,
>> struct sdap_handle *sh,
>> struct sdap_options *opts,
>> const char *username)
>> {
>> struct tevent_req *req;
>> struct tevent_req *subreq;
>> struct get_user_dn_state *state;
>> char *clean_name;
>> char *filter;
>> const char **attrs;
>> errno_t ret;
>>
>> req = tevent_req_create(memctx, &state, struct get_user_dn_state);
>> if (!req) return NULL;
>>
>> state->username = username;
>
> Input string should be strdupped to be safe, due to asynchronous
> nature of the request.
This is not necessary, get_user_dn_send is a specialized one-time
request and the username will always be allocated on top of the parent
request. We already do too many allocations.
>
>> static void get_user_dn_done(struct tevent_req *subreq)
>> {
>> errno_t ret;
>> struct tevent_req *req = tevent_req_callback_data(subreq,
>> struct tevent_req);
>> struct get_user_dn_state *state = tevent_req_data(req,
>> struct get_user_dn_state);
>> struct ldb_message_element *el;
>> struct sysdb_attrs **users;
>> size_t count;
>>
>> ret = sdap_search_user_recv(state, subreq, NULL, &users, &count);
>> talloc_zfree(subreq);
>> if (ret && ret != ENOENT) {
>> DEBUG(SSSDBG_OP_FAILURE, ("Failed to retrieve users\n"));
>> tevent_req_error(req, ret);
>> return;
>> }
>>
>> if (count == 0) {
>> DEBUG(SSSDBG_OP_FAILURE, ("No such user\n"));
>> tevent_req_error(req, ENOMEM);
>> return;
>> } else if (count > 1) {
>> DEBUG(SSSDBG_OP_FAILURE, ("Multiple users matched\n"));
>> tevent_req_error(req, EIO);
>> return;
>> }
>>
>> /* exactly one user. Get the originalDN */
>> ret = sysdb_attrs_get_el(users[0], SYSDB_ORIG_DN, &el);
>> if (ret || el == NULL || el->num_values == 0) {
>> DEBUG(SSSDBG_OP_FAILURE,
>> ("originalDN is not available for [%s].\n",
state->username));
>>
>> tevent_req_error(req, ret ? ret : EIO);
>> return;
>> }
>
> You can use directly sysdb_attrs_get_el_ext() with alloc param set
> to false. That will make error handling easier.
Done, the code indeed looks better now.
Ack.