Hey!
I'd do it a bit differently.
diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c index e44c045b3f8ff6aed33a42cf2919bc01aa41a243..3a8efa4caacbad74f493de334a387104d0e7cec4 100644 --- a/src/providers/ldap/sdap_async_users.c +++ b/src/providers/ldap/sdap_async_users.c @@ -519,6 +519,7 @@ int sdap_save_users(TALLOC_CTX *memctx, char **_usn_value) { TALLOC_CTX *tmpctx; + const char* user_name = NULL; char *higher_usn = NULL; char *usn_value; int ret; @@ -548,14 +549,22 @@ int sdap_save_users(TALLOC_CTX *memctx, for (i = 0; i < num_users; i++) { usn_value = NULL;
+ ret = sdap_get_user_primary_name(memctx, opts, users[i], + dom, &user_name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "Failed to get user name\n"); + goto done; + } +
IMO, if it fails, that's okay, then let's just go for using the loop index. IOW, please, remove the "goto done;" line.
ret = sdap_save_user(tmpctx, opts, dom, users[i], &usn_value, now);
/* Do not fail completely on errors. * Just report the failure to save and go on */ if (ret) { - DEBUG(SSSDBG_OP_FAILURE, "Failed to store user %d. Ignoring.\n", i); + DEBUG(SSSDBG_OP_FAILURE, "Failed to store user %s. Ignoring.\n", + user_name); } else { - DEBUG(SSSDBG_TRACE_ALL, "User %d processed!\n", i); + DEBUG(SSSDBG_TRACE_ALL, "User %s processed!\n", user_name); }
I didn't detailed check what sdap_get_user_primary_name() is doing, but I have the feeling that when it fails, user_name is NULL. So, when printing the debug message, you can check whether the username is not NULL and print it, otherwise you could print the array's index as it was done before your patch.
if (usn_value) { --
Reviewed-by: Fabiano Fidêncio fidencio@redhat.com
Best Regards,