[PATCH] sss_override: support fqn in override name

[Pavel Březina]

https://fedorahosted.org/sssd/ticket/2782

[Jakub Hrozek]

On Tue, Sep 15, 2015 at 11:39:37AM +0200, Pavel Březina wrote:
> https://fedorahosted.org/sssd/ticket/2782

Thanks, this patch seems to work well. See one question inline..
> From ce398000b0837013b4330260a3c093438a249f4a Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> Date: Tue, 15 Sep 2015 11:38:40 +0200
> Subject: [PATCH] sss_override: support fqn in override name
>
> Resolves:
> https://fedorahosted.org/sssd/ticket/2782
>
> src/tools/sss_override.c | 111 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 93 insertions(+), 18 deletions(-)

[...]
> static errno_t override_user(struct sss_tool_ctx *tool_ctx,
>                         struct override_user *user)
>                         struct override_user *input_user)
>
> {
> TALLOC_CTX *tmp_ctx;
> struct override_user user;
> struct sysdb_attrs *attrs;
> errno_t ret;
> ret = prepare_view_msg(user->domain);
> tmp_ctx = talloc_new(NULL);
> if (tmp_ctx == NULL) {
>    DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
>    return ENOMEM;
> }
> user = *input_user;

Why not use:
    struct override_user *user;
in the function? Wouldn't that allow us to keep using user-> instead of
user. and make the diff smaller?

Similar question for group.

[Pavel Březina]

On 09/16/2015 12:07 PM, Jakub Hrozek wrote:
> On Tue, Sep 15, 2015 at 11:39:37AM +0200, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/2782
>
> Thanks, this patch seems to work well. See one question inline..
> > From ce398000b0837013b4330260a3c093438a249f4a Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > Date: Tue, 15 Sep 2015 11:38:40 +0200
> > Subject: [PATCH] sss_override: support fqn in override name
> >
> > Resolves:
> > https://fedorahosted.org/sssd/ticket/2782
> >
> > src/tools/sss_override.c | 111 +++++++++++++++++++++++++++++++++++++++--------
> >   1 file changed, 93 insertions(+), 18 deletions(-)
>
> [...]
> > static errno_t override_user(struct sss_tool_ctx *tool_ctx,
> >                         struct override_user *user)
> >                         struct override_user *input_user)
> > {
> > TALLOC_CTX *tmp_ctx;
> > struct override_user user;
> >  struct sysdb_attrs *attrs;
> >  errno_t ret;
> > ret = prepare_view_msg(user->domain);
> > tmp_ctx = talloc_new(NULL);
> > if (tmp_ctx == NULL) {
> >    DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> >    return ENOMEM;
> > }
> > user = *input_user;
>
> Why not use:
>      struct override_user *user;
> in the function? Wouldn't that allow us to keep using user-> instead of
> user. and make the diff smaller?
>
> Similar question for group.

Because I'm changing value of user->name and I do not want to propagate 
this change to the caller, since it seems wrong to me.

[Jakub Hrozek]

On Wed, Sep 16, 2015 at 12:17:10PM +0200, Pavel Březina wrote:
> On 09/16/2015 12:07 PM, Jakub Hrozek wrote:
> > On Tue, Sep 15, 2015 at 11:39:37AM +0200, Pavel Březina wrote:
> > > https://fedorahosted.org/sssd/ticket/2782
> >
> > Thanks, this patch seems to work well. See one question inline..
> > > From ce398000b0837013b4330260a3c093438a249f4a Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
> > > Date: Tue, 15 Sep 2015 11:38:40 +0200
> > > Subject: [PATCH] sss_override: support fqn in override name
> > >
> > > Resolves:
> > > https://fedorahosted.org/sssd/ticket/2782
> > >
> > > src/tools/sss_override.c | 111 +++++++++++++++++++++++++++++++++++++++--------
> > >  1 file changed, 93 insertions(+), 18 deletions(-)
> >
> > [...]
> > > static errno_t override_user(struct sss_tool_ctx *tool_ctx,
> > >                         struct override_user *user)
> > >                         struct override_user *input_user)
> > >
> > > {
> > > TALLOC_CTX *tmp_ctx;
> > > struct override_user user;
> > > struct sysdb_attrs *attrs;
> > > errno_t ret;
> > > ret = prepare_view_msg(user->domain);
> > > tmp_ctx = talloc_new(NULL);
> > > if (tmp_ctx == NULL) {
> > >    DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed\n");
> > >    return ENOMEM;
> > > }
> > > user = *input_user;
> >
> > Why not use:
> >     struct override_user *user;
> > in the function? Wouldn't that allow us to keep using user-> instead of
> > user. and make the diff smaller?
> >
> > Similar question for group.
>
> Because I'm changing value of user->name and I do not want to propagate this
> change to the caller, since it seems wrong to me.

Ah, OK.

ACK

[Jakub Hrozek]

On Wed, Sep 16, 2015 at 12:19:31PM +0200, Jakub Hrozek wrote:
> Ah, OK.
>
> ACK

* master: 4649f19ea4b11c428ca75803beda8d495a0c9335