URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured

pbrezina commented:
"""
Well, currently I see some one kind of interesting code issue there that should 
be fixed as well.

Jakub did first this change:
```c
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 490) int sss_tool_main(int 
argc, const char **argv,
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 491)                   
struct sss_route_cmd *commands,
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 492)                   void 
*pvt)
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 493) {
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 494)     struct 
sss_tool_ctx *tool_ctx;
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 495)     uid_t uid;
e98ccef2 (Pavel Březina   2016-06-09 16:13:34 +0200 496)     errno_t ret;
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 497) 
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 498)     uid = getuid();
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 499)     if (uid != 0) {
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 500)         
DEBUG(SSSDBG_CRIT_FAILURE, "Running under %d, must be root\n", uid);
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 501)         ERROR("%1$s 
must be run as root\n", argv[0]);
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 502)         return 
EXIT_FAILURE;
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 503)     }
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 504) 
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 505)     ret = 
sss_tool_init(NULL, &argc, argv, &tool_ctx);
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 506)     if (ret == 
ERR_SYSDB_VERSION_TOO_OLD) {
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 507)         
tool_ctx->init_err = ret;
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 508)     } else if (ret != 
EOK) {
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 509)         
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create tool context\n");
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 510)         return 
EXIT_FAILURE;
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 511)     }
```

But then Michal removed the initialization code from `sss_tool_init` and put it 
to `tool_cmd_init` which is called from `sss_tool_route`.

```c
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 328)             if 
(!sss_tools_handles_init_error(&commands[i], tool_ctx->init_err)) {
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 329)                 
DEBUG(SSSDBG_FATAL_FAILURE,
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 330)                       
"Command %s does not handle initialization error [%d] %s\n",
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 331)                       
cmdline.command, tool_ctx->init_err,
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 332)                       
sss_strerror(tool_ctx->init_err));
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 333)                 return 
tool_ctx->init_err;
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 334)             }
a0b824ac (Jakub Hrozek    2016-07-01 13:26:38 +0200 335) 
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 336)             ret = 
tool_cmd_init(tool_ctx, &commands[i]);
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 337)             if (ret != 
EOK) {
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 338)                 
DEBUG(SSSDBG_FATAL_FAILURE,
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 339)                       
"Command initialization failed [%d] %s\n",
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 340)                       
ret, sss_strerror(ret));
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 341)                 return 
ret;
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 342)             }
cbee11e9 (Michal Židek    2016-10-12 13:09:37 +0200 343) 
284937e6 (Pavel Březina   2015-07-22 10:02:02 +0200 344)             return 
commands[i].fn(&cmdline, tool_ctx, pvt);

```

(I'm not pointing figers, I put git blame here just so we can see the git 
hash). This rendered Jakub's change basically a dead code, because 
`sss_tool_init` only returns `ENOMEM` or `EOK`.

Going back to this ticket. We parse command line for common options in 
`sss_tool_init`. We should also parse popt help options here, return some error 
code if help option was found and then skip the `tool_cmd_init`  or set 
`SSS_TOOL_FLAG_SKIP_CMD_INIT` on the comman(s).

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/515#issuecomment-367654935
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

Reply via email to