On 04/29/2015 07:00 PM, Lukas Slebodnik wrote:
On (29/04/15 12:21), Pavel Reichl wrote:
> Hello,
>
> please see attached patches, thanks!
>From e824d4aa1051bf405243deb06cef1e05ed3ffd25 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Fri, 27 Feb 2015 11:37:50 -0500
> Subject: [PATCH 1/3] krb5: new option krb5_map_user
>
> New option `krb5_map_user` providing mapping of ID provider names to
> Kerberos principals.
>
> Resolves:
>
https://fedorahosted.org/sssd/ticket/2509
> ---
> src/config/SSSDConfig/__init__.py.in | 1 +
> src/config/SSSDConfigTest.py | 9 ++-
> src/config/etc/sssd.api.d/sssd-ad.conf | 1 +
> src/config/etc/sssd.api.d/sssd-ipa.conf | 1 +
> src/config/etc/sssd.api.d/sssd-krb5.conf | 1 +
> src/man/sssd-krb5.5.xml | 26 ++++++++
> src/providers/ad/ad_opts.h | 1 +
> src/providers/ipa/ipa_opts.h | 1 +
> src/providers/krb5/krb5_access.c | 4 +-
> src/providers/krb5/krb5_auth.c | 40 +++++++++++-
> src/providers/krb5/krb5_auth.h | 2 +
> src/providers/krb5/krb5_common.h | 9 +++
> src/providers/krb5/krb5_init_shared.c | 101 +++++++++++++++++++++++++++++++
> src/providers/krb5/krb5_opts.h | 1 +
> src/util/util.c | 56 +++++++++++++++++
> src/util/util.h | 6 ++
> 16 files changed, 254 insertions(+), 6 deletions(-)
>
> diff --git a/src/util/util.c b/src/util/util.c
> index
613c559bb2002686c7833642d0946e46e5a9b5d6..a3da6c51d2b92a529b03632a26b6e07569b5da77 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -30,6 +30,62 @@
> #include "util/util.h"
> #include "util/sss_utf8.h"
>
> +errno_t split_str(TALLOC_CTX *mem_ctx,
> + const char delimiter,
> + char *str,
> + char **_first,
> + char **_second)
> +{
Why do we need new function split_str?
We already have function split_on_separator.
split_on_separator() does super set of
split_str(), code is IMO better
with split_str() which is sufficient here. In reviewed patch I used
both. Are you sure you don't see the difference?
The function split_on_separator is tested unlike yours function.
I'm sorry but utility function without unit test(s) is automatic NACK.
Please
could you point me to the source of such rules? I suppose other
developers would appreciate such list too as according to
https://jhrozek.fedorapeople.org/sssd-coverage/html/util/util.c.gcov.html there
are already functions without unit tests...how that could happen?
You should even have thought about sending such patch. :-)
I
agree that unit tests are cool and I would hate to lower code coverage
of util.c so please see updated patch set.
> + char *sep;
> + errno_t ret;
> + char *first;
> + char *second;
> + TALLOC_CTX *tmp_ctx;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (tmp_ctx == NULL) {
> + ret = ENOMEM;
> + goto done;
> + }
> +
> + sep = strchr(str, delimiter);
> + if (sep == NULL) {
> + ret = ENOENT;
> + goto done;
> + } else {
> + if (sep == str) {
> + first = NULL;
> + } else {
> + first = talloc_strndup(tmp_ctx, str, sep - str);
> + if (first == NULL) {
> + ret = ENOMEM;
> + goto done;
> + }
> + }
> +
> + if (*(sep + 1) == '\0') {
> + second = NULL;
> + } else {
> + second = talloc_strdup(tmp_ctx, sep+1);
> + if (second == NULL) {
> + ret = ENOMEM;
> + goto done;
> + }
> + }
> + }
> +
> + ret = EOK;
> +
> +done:
> + if (ret == EOK) {
> + *_first = talloc_steal(mem_ctx, first);
> + *_second = talloc_steal(mem_ctx, second);
> + }
> +
> + talloc_free(tmp_ctx);
> + return ret;
> +}
> +
> int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
> const char sep, bool trim, bool skip_empty,
> char ***_list, int *size)
> diff --git a/src/util/util.h b/src/util/util.h
> index
3c5ce295ec4ad2ad3c3a7ae0e7482c232db79746..479e24cabe947419cf9d63e803b1b662359ad38b 100644
> --- a/src/util/util.h
> +++ b/src/util/util.h
> @@ -408,6 +408,12 @@ const char * const * get_known_services(void);
>
> errno_t sss_user_by_name_or_uid(const char *input, uid_t *_uid, gid_t *_gid);
>
> +errno_t split_str(TALLOC_CTX *mem_ctx,
> + const char delimiter,
> + char *str,
> + char **_first,
> + char **_second);
> +
> int split_on_separator(TALLOC_CTX *mem_ctx, const char *str,
> const char sep, bool trim, bool skip_empty,
> char ***_list, int *size);
> --
> 2.1.0
>
>From 50839ec51277cb9aa45c21b71bfb9dc5e8b0b553 Mon Sep 17 00:00:00 2001
> From: Pavel Reichl <preichl(a)redhat.com>
> Date: Wed, 29 Apr 2015 06:03:04 -0400
> Subject: [PATCH 2/3] krb5: remove field run_as_user
>
> run_as_user is set set but never read.
> ---
LGTM
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel