[SSSD] [PATCH] idmap: Silence DEBUG messages when dealing with built-in, SIDs.
Pavel Březina
pbrezina at redhat.com
Tue Nov 20 14:20:06 UTC 2012
On 11/15/2012 09:11 PM, Michal Židek wrote:
> https://fedorahosted.org/sssd/ticket/1593
>
> With this patch we get:
>
> [sssd[be[AD]]] [sdap_save_group] (0x1000): Mapping group
> [Administrators] objectSID to unix ID
> [sssd[be[AD]]] [sdap_idmap_sid_to_unix] (0x0400): Object SID
> [S-1-5-32-544] is a built-in one.
> [sssd[be[AD]]] [sdap_save_group] (0x0400): Skipping built-in object.
>
> Instead of the old messages, that looked like this:
>
> [sssd[be[AD]]] [sdap_save_group] (0x1000): Mapping group
> [Administrators] objectSID to unix ID
> [sssd[be[AD]]] [sdap_idmap_sid_to_unix] (0x0080): Could not parse domain
> SID from [S-1-5-32-544]
> [sssd[be[AD]]] [sdap_save_group] (0x0080): Could not convert SID string:
> [Invalid argument]
> [sssd[be[AD]]] [sdap_save_group] (0x0080): Failed to save group
> [Administrators]: [Invalid argument]
> [sssd[be[AD]]] [sdap_save_groups] (0x0040): Failed to store group 0.
> Ignoring.
>
>
> The patch is attached.
>
> NOTE: To reproduce, set enumerate = true for the AD domain, delete the
> cache files and start SSSD.
>
> Thanks
> Michal
Nack.
> +static bool sss_idmap_sid_is_builtin(const char *sid)
> +{
> + if (strncmp(sid, "S-1-5-32-", 9)) {
> + return false;
> + } else {
> + return true;
> + }
> +}
Please, change ^^ to:
+static bool sss_idmap_sid_is_builtin(const char *sid)
+{
+ if (strncmp(sid, "S-1-5-32-", 9) == 0) {
+ return true;
+ }
+
+ return false;
+}
> @@ -380,7 +380,8 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx *idmap_ctx,
> err = sss_idmap_sid_to_unix(idmap_ctx->map,
> sid_str,
> (uint32_t *)id);
> - if (err != IDMAP_SUCCESS && err != IDMAP_NO_DOMAIN) {
> + if (err != IDMAP_SUCCESS && err != IDMAP_NO_DOMAIN &&
> + err != IDMAP_BUILTIN_SID) {
> DEBUG(SSSDBG_MINOR_FAILURE,
> ("Could not convert objectSID [%s] to a UNIX ID\n",
> sid_str));
> @@ -420,6 +421,10 @@ sdap_idmap_sid_to_unix(struct sdap_idmap_ctx *idmap_ctx,
> ret = EIO;
> goto done;
> }
> + } else if (err == IDMAP_BUILTIN_SID) {
Can you use switch statement please? Like:
switch (err) {
case IDMAP_NO_DOMAIN:
...
break;
case IDMAP_BUILTIN_SID:
...
break;
default:
error
}
> + DEBUG(SSSDBG_TRACE_FUNC,
> + ("Object SID [%s] is a built-in one.\n", sid_str));
> + *id = 0; /* O indicates, that this ID should be ignored */
>
We should propagate the built-in sid error instead of misusing id. Maybe
return IDMAP* directly and return errno value in new output parameter.
Thanks,
Pavel.
More information about the sssd-devel
mailing list