[PATCH v3] fixes account locked when using winbind refresh tickets

Ralph Böhme slow at samba.org
Mon Jan 15 15:11:14 UTC 2018


Hi David,

On Mon, Jan 15, 2018 at 07:52:40AM -0700, David Mulder wrote:
> Touch ups recommended by Andreas (null initialize, helper variables, etc).

thanks, much better! :)

Can you please also simplify a few if conditions and use early exits, eg you
have

+       at_ptr = strchr(entry->principal_name, '@');
+       if (at_ptr != NULL) {
+               int strlen = at_ptr - entry->principal_name;
+               sam = talloc_strndup(ctx, entry->principal_name, strlen);
+       } else {
+               DBG_DEBUG("Could not determine samAccountName from %s\n",
+                         entry->principal_name);
+               goto fail;
+       }

Simpler:

+       int strlen;
        ...
+       at_ptr = strchr(entry->principal_name, '@');
+       if (at_ptr == NULL) {
+               DBG_DEBUG("Could not determine samAccountName from %s\n",
+                         entry->principal_name);
+               goto fail;
+       }
+
+       strlen = at_ptr - entry->principal_name;
+       sam = talloc_strndup(ctx, entry->principal_name, strlen);

Similar for the hunk that adds ads_do_search_all().

I guess is too much asking for a refactoring of krb5_ticket_refresh_handler()
as part of the fix? This function is pure horror.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/



More information about the samba-technical mailing list