>From 136fe621d7f87fa398d15e436dd33e330bca86b6 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Fri, 28 Aug 2009 18:20:44 +0200
Subject: [PATCH 1/3] Refactor tools code

Move parameter parsing in tools before attempting to do anything that
might fail - so that we have debug_level set correctly for potential
error messages. That allows printing the --help and --usage messages
without being root.

Fix code duplicates in tools and refactor its code a little to lay
ground for decoupling the synchronous interfaces.

Remove some legacy tools leftovers, re-add sensible error message on
removing nonexistent users/groups which was removed by accident.

Fixes: Trac ticket #75
Fix typo in groupdel: fixes ticket #136
---
 server/tools/sss_groupadd.c |   44 +++++---------
 server/tools/sss_groupdel.c |   56 +++++++++----------
 server/tools/sss_groupmod.c |  103 +++++++++++++++++++---------------
 server/tools/sss_useradd.c  |   61 +++++++++-----------
 server/tools/sss_userdel.c  |   52 ++++++++---------
 server/tools/sss_usermod.c  |  128 ++++++++++++++++++++++++------------------
 server/tools/tools_util.c   |   32 ++++++-----
 server/tools/tools_util.h   |   30 +----------
 8 files changed, 242 insertions(+), 264 deletions(-)

diff --git a/server/tools/sss_groupadd.c b/server/tools/sss_groupadd.c
index d0c0283..830ba79 100644
--- a/server/tools/sss_groupadd.c
+++ b/server/tools/sss_groupadd.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <unistd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 
 #include "util/util.h"
 #include "db/sysdb.h"
@@ -48,7 +46,7 @@ static void add_group_terminate(struct ops_ctx *data, int error)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -78,7 +76,7 @@ static void add_group(struct tevent_req *req)
         return add_group_terminate(data, ret);
     }
 
-    subreq = sysdb_add_group_send(data, data->ev, data->handle,
+    subreq = sysdb_add_group_send(data, data->ctx->ev, data->handle,
                                   data->domain, data->name,
                                   data->gid, NULL);
     if (!subreq) {
@@ -111,7 +109,6 @@ int main(int argc, const char **argv)
         POPT_TABLEEND
     };
     poptContext pc = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     struct ops_ctx *data = NULL;
     int ret = EXIT_SUCCESS;
@@ -126,29 +123,11 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory.\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
     /* parse params */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "GROUPNAME");
-    if((ret = poptGetNextOpt(pc)) < -1) {
+    if ((ret = poptGetNextOpt(pc)) < -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
         goto fini;
@@ -164,6 +143,16 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = get_domain(data, pc_groupname);
     if (ret != EOK) {
         ERROR("Cannot get domain information\n");
@@ -181,7 +170,7 @@ int main(int argc, const char **argv)
     }
 
     /* add_group */
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not add group.\n");
@@ -191,14 +180,14 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, add_group, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
         ret = data->error;
         switch (ret) {
             case EEXIST:
-                ERROR("A group with the same name or UID already exists\n");
+                ERROR("A group with the same name or GID already exists\n");
                 break;
 
             default:
@@ -213,7 +202,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 fini:
     talloc_free(data);
-    talloc_free(ctx);
     poptFreeContext(pc);
     exit(ret);
 }
diff --git a/server/tools/sss_groupdel.c b/server/tools/sss_groupdel.c
index 74d3071..7d8f541 100644
--- a/server/tools/sss_groupdel.c
+++ b/server/tools/sss_groupdel.c
@@ -24,8 +24,6 @@
 #include <talloc.h>
 #include <popt.h>
 #include <grp.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "db/sysdb.h"
@@ -50,7 +48,7 @@ static void groupdel_done(void *pvt, int error, struct ldb_result *ignore)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -88,7 +86,7 @@ static void group_del(struct tevent_req *req)
         return groupdel_done(data, ENOMEM, NULL);
     }
 
-    subreq = sysdb_delete_entry_send(data, data->ev, data->handle, group_dn, false);
+    subreq = sysdb_delete_entry_send(data, data->ctx->ev, data->handle, group_dn, false);
     if (!subreq)
         return groupdel_done(data, ENOMEM, NULL);
 
@@ -110,7 +108,6 @@ int main(int argc, const char **argv)
     int ret = EXIT_SUCCESS;
     int pc_debug = 0;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     struct group *grp_info;
     const char *pc_groupname = NULL;
@@ -132,29 +129,11 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
     /* parse ops_ctx */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
-    poptSetOtherOptionHelp(pc, "USERNAME");
-    if((ret = poptGetNextOpt(pc)) < -1) {
+    poptSetOtherOptionHelp(pc, "GROUPNAME");
+    if ((ret = poptGetNextOpt(pc)) < -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
         goto fini;
@@ -163,12 +142,22 @@ int main(int argc, const char **argv)
     debug_level = pc_debug;
 
     pc_groupname = poptGetArg(pc);
-    if(pc_groupname == NULL) {
+    if (pc_groupname == NULL) {
         usage(pc, _("Specify group to delete\n"));
         ret = EXIT_FAILURE;
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     /* if the domain was not given as part of FQDN, default to local domain */
     ret = get_domain(data, pc_groupname);
     if (ret != EOK) {
@@ -191,7 +180,7 @@ int main(int argc, const char **argv)
     }
 
     /* groupdel */
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not remove group.\n");
@@ -201,13 +190,21 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, group_del, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
         ret = data->error;
         DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret)));
-        ERROR("Internal error. Could not remove group.\n");
+        switch (ret) {
+            case ENOENT:
+                ERROR("No such group\n");
+                break;
+
+            default:
+                ERROR("Internal error. Could not remove group.\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
@@ -215,7 +212,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
-    talloc_free(ctx);
     talloc_free(data);
     poptFreeContext(pc);
     exit(ret);
diff --git a/server/tools/sss_groupmod.c b/server/tools/sss_groupmod.c
index 464c165..cc1a945 100644
--- a/server/tools/sss_groupmod.c
+++ b/server/tools/sss_groupmod.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <grp.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -49,7 +47,7 @@ static void mod_group_done(struct ops_ctx *data, int error)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -98,7 +96,7 @@ static void mod_group(struct tevent_req *req)
             mod_group_done(data, ret);
         }
 
-        subreq = sysdb_set_group_attr_send(data, data->ev, data->handle,
+        subreq = sysdb_set_group_attr_send(data, data->ctx->ev, data->handle,
                                            data->domain, data->name,
                                            attrs, SYSDB_MOD_REP);
         if (!subreq) {
@@ -158,7 +156,7 @@ static void remove_from_groups(struct ops_ctx *data)
     }
 
     req = sysdb_mod_group_member_send(data,
-                                      data->ev,
+                                      data->ctx->ev,
                                       data->handle,
                                       member_dn,
                                       parent_dn,
@@ -215,7 +213,7 @@ static void add_to_groups(struct ops_ctx *data)
     }
 
     req = sysdb_mod_group_member_send(data,
-                                      data->ev,
+                                      data->ctx->ev,
                                       data->handle,
                                       member_dn,
                                       parent_dn,
@@ -266,9 +264,8 @@ int main(int argc, const char **argv)
     };
     poptContext pc = NULL;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
-    char *groups;
+    char *addgroups = NULL, *rmgroups = NULL;
     int ret;
     struct group *grp_info;
     gid_t old_gid = 0;
@@ -283,49 +280,28 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
 
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
-    poptSetOtherOptionHelp(pc, "USERNAME");
+    poptSetOtherOptionHelp(pc, "GROUPNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
-        if (ret == 'a' || ret == 'r') {
-            groups = poptGetOptArg(pc);
-            if (!groups) {
-                ret = -1;
+        switch (ret) {
+            case 'a':
+                addgroups = poptGetOptArg(pc);
+                if (addgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
-
-            ret = parse_groups(ctx,
-                    groups,
-                    (ret == 'a') ? (&data->addgroups) : (&data->rmgroups));
 
-            free(groups);
-            if (ret != EOK) {
+            case 'r':
+                rmgroups = poptGetOptArg(pc);
+                if (rmgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
         }
     }
 
-    debug_level = pc_debug;
-
     if (ret != -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
@@ -340,7 +316,18 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    /* if the domain was not given as part of FQDN, default to local domain */
+    debug_level = pc_debug;
+
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     ret = get_domain(data, pc_groupname);
     if (ret != EOK) {
         ERROR("Cannot get domain information\n");
@@ -350,6 +337,28 @@ int main(int argc, const char **argv)
 
     data->gid = pc_gid;
 
+    if (addgroups) {
+        ret = parse_groups(data,
+                addgroups,
+                &data->addgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the group to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
+    if (rmgroups) {
+        ret = parse_groups(data,
+                rmgroups,
+                &data->rmgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to remove the group from\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
     /* arguments processed, go on to actual work */
     grp_info = getgrnam(data->name);
     if (grp_info) {
@@ -362,7 +371,7 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify group.\n");
@@ -372,7 +381,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, mod_group, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -399,7 +408,9 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     exit(ret);
 }
diff --git a/server/tools/sss_useradd.c b/server/tools/sss_useradd.c
index f621036..2ce1607 100644
--- a/server/tools/sss_useradd.c
+++ b/server/tools/sss_useradd.c
@@ -25,8 +25,6 @@
 #include <talloc.h>
 #include <popt.h>
 #include <errno.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -125,7 +123,7 @@ static void add_user_terminate(struct ops_ctx *data, int error)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -157,7 +155,7 @@ static void add_user(struct tevent_req *req)
         return add_user_terminate(data, ret);
     }
 
-    subreq = sysdb_add_user_send(data, data->ev, data->handle,
+    subreq = sysdb_add_user_send(data, data->ctx->ev, data->handle,
                                  data->domain, data->name,
                                  data->uid, data->gid,
                                  data->gecos, data->home,
@@ -205,7 +203,7 @@ static void add_to_groups(struct ops_ctx *data)
         return add_user_terminate(data, ENOMEM);
     }
 
-    subreq = sysdb_mod_group_member_send(data, data->ev, data->handle,
+    subreq = sysdb_mod_group_member_send(data, data->ctx->ev, data->handle,
                                          member_dn, parent_dn,
                                          LDB_FLAG_MOD_ADD);
     if (!subreq) {
@@ -259,7 +257,6 @@ int main(int argc, const char **argv)
     };
     poptContext pc = NULL;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     char *groups = NULL;
     int ret;
@@ -273,26 +270,8 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for ops_ctx context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
@@ -302,17 +281,12 @@ int main(int argc, const char **argv)
                 ret = -1;
                 break;
             }
-
-            ret = parse_groups(ctx, groups, &data->groups);
-            if (ret != EOK) {
-                break;
-            }
         }
     }
 
     debug_level = pc_debug;
 
-    if(ret != -1) {
+    if (ret != -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
         goto fini;
@@ -326,6 +300,16 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     /* if the domain was not given as part of FQDN, default to local domain */
     ret = get_domain(data, pc_username);
     if (ret != EOK) {
@@ -334,6 +318,15 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    if (groups) {
+        ret = parse_groups(data, groups, &data->groups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the user to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
     /* Same as shadow-utils useradd, -g can specify gid or group name */
     if (pc_group != NULL) {
         ret = get_gid(data, pc_group);
@@ -403,7 +396,7 @@ int main(int argc, const char **argv)
     }
 
     /* useradd */
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify user.\n");
@@ -413,7 +406,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, add_user, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -437,7 +430,7 @@ int main(int argc, const char **argv)
 
 fini:
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     free(groups);
     exit(ret);
 }
diff --git a/server/tools/sss_userdel.c b/server/tools/sss_userdel.c
index 00ccc86..ead6188 100644
--- a/server/tools/sss_userdel.c
+++ b/server/tools/sss_userdel.c
@@ -23,8 +23,6 @@
 #include <stdlib.h>
 #include <talloc.h>
 #include <popt.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <pwd.h>
 #include <unistd.h>
 
@@ -50,7 +48,7 @@ static void userdel_done(void *pvt, int error, struct ldb_result *ignore)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -90,7 +88,7 @@ static void user_del(struct tevent_req *req)
         return userdel_done(data, ENOMEM, NULL);
     }
 
-    subreq = sysdb_delete_entry_send(data, data->ev, data->handle, user_dn, false);
+    subreq = sysdb_delete_entry_send(data, data->ctx->ev, data->handle, user_dn, false);
     if (!subreq)
         return userdel_done(data, ENOMEM, NULL);
 
@@ -111,7 +109,6 @@ int main(int argc, const char **argv)
 {
     int ret = EXIT_SUCCESS;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
     struct passwd *pwd_info;
     const char *pc_username = NULL;
@@ -134,26 +131,8 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
-
-    ret = init_sss_tools(&ctx);
-    if(ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(NULL, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
 
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     if ((ret = poptGetNextOpt(pc)) < -1) {
@@ -171,6 +150,16 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
     /* if the domain was not given as part of FQDN, default to local domain */
     ret = get_domain(data, pc_username);
     if (ret != EOK) {
@@ -192,7 +181,7 @@ int main(int argc, const char **argv)
     }
 
     /* userdel */
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not remove user.\n");
@@ -202,13 +191,21 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, user_del, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
         ret = data->error;
         DEBUG(1, ("sysdb operation failed (%d)[%s]\n", ret, strerror(ret)));
-        ERROR("Internal error. Could not remove user.\n");
+        switch (ret) {
+            case ENOENT:
+                ERROR("No such user\n");
+                break;
+
+            default:
+                ERROR("Internal error. Could not remove user.\n");
+                break;
+        }
         ret = EXIT_FAILURE;
         goto fini;
     }
@@ -216,7 +213,6 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
-    talloc_free(ctx);
     talloc_free(data);
     poptFreeContext(pc);
     exit(ret);
diff --git a/server/tools/sss_usermod.c b/server/tools/sss_usermod.c
index a49dc8e..b410ed1 100644
--- a/server/tools/sss_usermod.c
+++ b/server/tools/sss_usermod.c
@@ -25,8 +25,6 @@
 #include <popt.h>
 #include <errno.h>
 #include <pwd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "util/util.h"
@@ -61,7 +59,7 @@ static void mod_user_done(struct ops_ctx *data, int error)
         goto fail;
     }
 
-    req = sysdb_transaction_commit_send(data, data->ev, data->handle);
+    req = sysdb_transaction_commit_send(data, data->ctx->ev, data->handle);
     if (!req) {
         error = ENOMEM;
         goto fail;
@@ -99,7 +97,7 @@ static void mod_user(struct tevent_req *req)
     }
 
     if (data->attrs->num != 0) {
-        subreq = sysdb_set_user_attr_send(data, data->ev, data->handle,
+        subreq = sysdb_set_user_attr_send(data, data->ctx->ev, data->handle,
                                           data->domain, data->name,
                                           data->attrs, SYSDB_MOD_REP);
         if (!subreq) {
@@ -159,7 +157,7 @@ static void remove_from_groups(struct ops_ctx *data)
     }
 
     req = sysdb_mod_group_member_send(data,
-                                      data->ev,
+                                      data->ctx->ev,
                                       data->handle,
                                       member_dn,
                                       parent_dn,
@@ -216,7 +214,7 @@ static void add_to_groups(struct ops_ctx *data)
     }
 
     req = sysdb_mod_group_member_send(data,
-                                      data->ev,
+                                      data->ctx->ev,
                                       data->handle,
                                       member_dn,
                                       parent_dn,
@@ -274,9 +272,8 @@ int main(int argc, const char **argv)
     };
     poptContext pc = NULL;
     struct ops_ctx *data = NULL;
-    struct tools_ctx *ctx = NULL;
     struct tevent_req *req;
-    char *groups;
+    char *addgroups = NULL, *rmgroups = NULL;
     int ret;
     struct passwd *pwd_info;
     uid_t old_uid = 0;
@@ -291,66 +288,44 @@ int main(int argc, const char **argv)
         ret = EXIT_FAILURE;
         goto fini;
     }
-    CHECK_ROOT(ret, debug_prg_name);
 
-    ret = init_sss_tools(&ctx);
-    if (ret != EOK) {
-        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
-        ERROR("Error initializing the tools\n");
-        ret = EXIT_FAILURE;
-        goto fini;
-    }
-
-    data = talloc_zero(ctx, struct ops_ctx);
-    if (data == NULL) {
-        DEBUG(1, ("Could not allocate memory for data context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-    data->ctx = ctx;
-    data->ev = ctx->ev;
-
-    data->attrs = sysdb_new_attrs(ctx);
-    if (data->attrs == NULL) {
-        DEBUG(1, ("Could not allocate memory for sysdb_attrs context\n"));
-        ERROR("Out of memory\n");
-        return ENOMEM;
-    }
-
-    /* parse ops_ctx */
+    /* parse parameters */
     pc = poptGetContext(NULL, argc, argv, long_options, 0);
     poptSetOtherOptionHelp(pc, "USERNAME");
     while ((ret = poptGetNextOpt(pc)) > 0) {
-        if (ret == 'a' || ret == 'r') {
-            groups = poptGetOptArg(pc);
-            if (!groups) {
-                ret = -1;
+        switch (ret) {
+            case 'a':
+                addgroups = poptGetOptArg(pc);
+                if (addgroups == NULL) {
+                    ret = -1;
+                }
                 break;
-            }
 
-            ret = parse_groups(ctx,
-                    groups,
-                    (ret == 'a') ? (&data->addgroups) : (&data->rmgroups));
+            case 'r':
+                rmgroups = poptGetOptArg(pc);
+                if (rmgroups == NULL) {
+                    ret = -1;
+                }
+                break;
+
+            case 'L':
+                pc_lock = DO_LOCK;
+                break;
 
-            free(groups);
-            if (ret != EOK) {
+            case 'U':
+                pc_lock = DO_UNLOCK;
                 break;
-            }
-        } else if (ret == 'L') {
-            pc_lock = DO_LOCK;
-        } else if (ret == 'U') {
-            pc_lock = DO_UNLOCK;
         }
     }
 
-    debug_level = pc_debug;
-
-    if(ret != -1) {
+    if (ret != -1) {
         usage(pc, poptStrerror(ret));
         ret = EXIT_FAILURE;
         goto fini;
     }
 
+    debug_level = pc_debug;
+
     /* username is an argument without --option */
     pc_username = poptGetArg(pc);
     if (pc_username == NULL) {
@@ -359,6 +334,23 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    CHECK_ROOT(ret, debug_prg_name);
+
+    ret = init_sss_tools(&data);
+    if (ret != EOK) {
+        DEBUG(1, ("init_sss_tools failed (%d): %s\n", ret, strerror(ret)));
+        ERROR("Error initializing the tools\n");
+        ret = EXIT_FAILURE;
+        goto fini;
+    }
+
+    data->attrs = sysdb_new_attrs(data->ctx);
+    if (data->attrs == NULL) {
+        DEBUG(1, ("Could not allocate memory for sysdb_attrs context\n"));
+        ERROR("Out of memory\n");
+        return ENOMEM;
+    }
+
     /* if the domain was not given as part of FQDN, default to local domain */
     ret = get_domain(data, pc_username);
     if (ret != EOK) {
@@ -378,6 +370,28 @@ int main(int argc, const char **argv)
         goto fini;
     }
 
+    if (addgroups) {
+        ret = parse_groups(data,
+                           addgroups,
+                           &data->addgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to add the user to\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
+    if (rmgroups) {
+        ret = parse_groups(data,
+                           rmgroups,
+                           &data->rmgroups);
+        if (ret != EOK) {
+            DEBUG(1, ("Cannot parse groups to remove the user from\n"));
+            ERROR("Internal error while parsing parameters\n");
+            goto fini;
+        }
+    }
+
     /* add parameters to changeset */
     /* FIXME - might want to do this via attr:pc_var mapping in a loop */
 
@@ -413,6 +427,7 @@ int main(int argc, const char **argv)
                   "Could not add attribute to changeset\n");
     }
 
+
     if(pc_gid) {
         ret = sysdb_attrs_add_long(data->attrs,
                                    SYSDB_GIDNUM,
@@ -438,7 +453,8 @@ int main(int argc, const char **argv)
                   "Could not add attribute to changeset\n");
     }
 
-    req = sysdb_transaction_send(ctx, ctx->ev, data->ctx->sysdb);
+
+    req = sysdb_transaction_send(data, data->ctx->ev, data->ctx->sysdb);
     if (!req) {
         DEBUG(1, ("Could not start transaction (%d)[%s]\n", ret, strerror(ret)));
         ERROR("Transaction error. Could not modify user.\n");
@@ -448,7 +464,7 @@ int main(int argc, const char **argv)
     tevent_req_set_callback(req, mod_user, data);
 
     while (!data->done) {
-        tevent_loop_once(ctx->ev);
+        tevent_loop_once(data->ctx->ev);
     }
 
     if (data->error) {
@@ -476,7 +492,9 @@ int main(int argc, const char **argv)
     ret = EXIT_SUCCESS;
 
 fini:
+    free(addgroups);
+    free(rmgroups);
     poptFreeContext(pc);
-    talloc_free(ctx);
+    talloc_free(data);
     exit(ret);
 }
diff --git a/server/tools/tools_util.c b/server/tools/tools_util.c
index 15ba16f..8e8ba66 100644
--- a/server/tools/tools_util.c
+++ b/server/tools/tools_util.c
@@ -45,10 +45,9 @@ static struct sss_domain_info *get_local_domain(struct tools_ctx *ctx)
 
 int setup_db(struct tools_ctx **tools_ctx)
 {
-    struct sss_domain_info *dom;
-    TALLOC_CTX *tmp_ctx;
     char *confdb_path;
     struct tools_ctx *ctx;
+    struct sss_domain_info *dom = NULL;
     int ret;
 
     ctx = talloc_zero(NULL, struct tools_ctx);
@@ -65,11 +64,7 @@ int setup_db(struct tools_ctx **tools_ctx)
         return EIO;
     }
 
-    tmp_ctx = talloc_new(ctx);
-    if (!tmp_ctx)
-        return ENOMEM;
-
-    confdb_path = talloc_asprintf(tmp_ctx, "%s/%s", DB_PATH, CONFDB_FILE);
+    confdb_path = talloc_asprintf(ctx, "%s/%s", DB_PATH, CONFDB_FILE);
     if (confdb_path == NULL) {
         talloc_free(ctx);
         return ENOMEM;
@@ -105,7 +100,7 @@ int setup_db(struct tools_ctx **tools_ctx)
         return ret;
     }
 
-    talloc_free(tmp_ctx);
+    talloc_free(confdb_path);
     *tools_ctx = ctx;
     return EOK;
 }
@@ -119,7 +114,6 @@ void usage(poptContext pc, const char *error)
     if (error) fprintf(stderr, "%s", error);
 }
 
-/* FIXME: avoid using strtok !! */
 int parse_groups(TALLOC_CTX *mem_ctx, const char *optstr, char ***_out)
 {
     char **out;
@@ -252,26 +246,36 @@ int set_locale(void)
     return EOK;
 }
 
-int init_sss_tools(struct tools_ctx **_ctx)
+int init_sss_tools(struct ops_ctx **_octx)
 {
     int ret;
-    struct tools_ctx *ctx;
+    struct tools_ctx *tctx;
+    struct ops_ctx *octx;
 
     /* Connect to the database */
-    ret = setup_db(&ctx);
+    ret = setup_db(&tctx);
     if (ret != EOK) {
         DEBUG(1, ("Could not set up database\n"));
         ret = EXIT_FAILURE;
         goto fini;
     }
 
-    ret = sss_names_init(ctx, ctx->confdb, &ctx->snctx);
+    ret = sss_names_init(tctx, tctx->confdb, &tctx->snctx);
     if (ret != EOK) {
         DEBUG(1, ("Could not set up parsing\n"));
         goto fini;
     }
 
-    *_ctx = ctx;
+    octx = talloc_zero(tctx, struct ops_ctx);
+    if (octx == NULL) {
+        DEBUG(1, ("Could not allocate memory for data context\n"));
+        ERROR("Out of memory\n");
+        ret = ENOMEM;
+        goto fini;
+    }
+    octx->ctx = tctx;
+
+    *_octx = octx;
     ret = EOK;
 fini:
     return ret;
diff --git a/server/tools/tools_util.h b/server/tools/tools_util.h
index 8cbbc7e..4718754 100644
--- a/server/tools/tools_util.h
+++ b/server/tools/tools_util.h
@@ -25,25 +25,6 @@
 
 #include "util/sssd-i18n.h"
 
-#define UID_NOT_SET 0
-#define GID_NOT_SET 0
-
-#define APPEND_PARAM(str, param, val) do { \
-    if (val) { \
-        str = talloc_asprintf_append(str, param, val); \
-        if (str == NULL) { \
-            return ENOMEM; \
-        } \
-    } \
-} while(0)
-
-#define APPEND_STRING(str, val) do { \
-    str = talloc_asprintf_append(str, "%s ", val); \
-    if (str == NULL) { \
-        return ENOMEM; \
-    } \
-} while(0)
-
 #define CHECK_ROOT(val, prg_name) do { \
     val = getuid(); \
     if (val != 0) { \
@@ -54,14 +35,6 @@
     } \
 } while(0)
 
-enum id_domain {
-    ID_IN_LOCAL = 0,
-    ID_IN_LEGACY_LOCAL,
-    ID_IN_OTHER,
-    ID_OUTSIDE,
-    ID_ERROR
-};
-
 struct tools_ctx {
     struct tevent_context *ev;
     struct confdb_ctx *confdb;
@@ -73,7 +46,6 @@ struct tools_ctx {
 
 struct ops_ctx {
     struct tools_ctx *ctx;
-    struct tevent_context *ev;
     struct sss_domain_info *domain;
 
     char *name;
@@ -94,7 +66,7 @@ struct ops_ctx {
     bool done;
 };
 
-int init_sss_tools(struct tools_ctx **_ctx);
+int init_sss_tools(struct ops_ctx **_octx);
 
 int setup_db(struct tools_ctx **ctx);
 
-- 
1.6.2.5

