On 04/19/2016 09:59 AM, Lukas Slebodnik wrote:
On (01/03/16 13:53), Pavel Březina wrote:
> On 02/25/2016 02:08 PM, Jakub Hrozek wrote:
>> On Tue, Feb 09, 2016 at 02:07:21PM +0100, Pavel Březina wrote:
>>> First of the responders is converted -)
>>
>> Sorry for the first delay in review. Before reading the code, I
>> submitted the patches to automated tests, so far I found:
>>
>> Error: COMPILER_WARNING:
>> sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c:29: included_from:
Included from here.
>> sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c: scope_hint: In function
'sudosrv_get_rules_done'
>> sssd-1.13.90/src/util/util.h:144:9: warning: 'debug_name' may be used
uninitialized in this function [-Wmaybe-uninitialized]
>> # sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \
>> # ^
>> sssd-1.13.90/src/responder/sudo/sudosrv_get_sudorules.c:225:17: note:
'debug_name' was declared here
>> # const char *debug_name;
>> # ^
>> # 142| int __debug_macro_level = level; \
>> # 143| if (DEBUG_IS_SET(__debug_macro_level)) { \
>> # 144|-> sss_debug_fn(__FILE__, __LINE__, __FUNCTION__, \
>> # 145| __debug_macro_level, \
>> # 146| format, ##__VA_ARGS__); \
>>
>> But feel free to only fix the issue in your branch for now, at least until
>> all the automated downstream tests finish.
>
// snip
> +
> +static errno_t sudosrv_query_cache(TALLOC_CTX *mem_ctx,
> + struct sss_domain_info *domain,
> + const char **attrs,
> + unsigned int flags,
> + const char *username,
> + uid_t uid,
> + char **groupnames,
> + bool inverse_order,
> + struct sysdb_attrs ***_rules,
> + uint32_t *_count)
> +{
> + TALLOC_CTX *tmp_ctx;
> + char *filter;
> + errno_t ret;
> + size_t count;
> + struct sysdb_attrs **rules;
> + struct ldb_message **msgs;
> +
> + tmp_ctx = talloc_new(NULL);
> + if (tmp_ctx == NULL) {
> + return ENOMEM;
> + }
> +
> + ret = sysdb_get_sudo_filter(tmp_ctx, username, uid, groupnames,
> + flags, &filter);
> + if (ret != EOK) {
> + DEBUG(SSSDBG_CRIT_FAILURE, "Could not construct the search filter
"
> + "[%d]: %s\n", ret, sss_strerror(ret));
> + goto done;
> + }
> +
> + DEBUG(SSSDBG_FUNC_DATA, "Searching sysdb with [%s]\n", filter);
> +
> + if (IS_SUBDOMAIN(domain)) {
> + /* rules are stored inside parent domain tree */
> + domain = domain->parent;
> + }
> +
> + ret = sysdb_search_custom(tmp_ctx, domain, filter, SUDORULE_SUBDIR,
> + attrs, &count, &msgs);
> + if (ret != EOK && ret != ENOENT) {
> + DEBUG(SSSDBG_CRIT_FAILURE, "Error looking up SUDO rules\n");
> + goto done;
> + } if (ret == ENOENT) {
^^^^^^^^^^^^^^^^^^^^^^
I wanted to push these ACK-ed patches but I noticed
this non-standard change :-)
I think you wanted here "else if" or "if" on next line.
I can change it before push If you let me which way you like more.
ret = sysdb_search_custom(tmp_ctx, domain, filter, SUDORULE_SUBDIR,
attrs, &count, &msgs);
if (ret == ENOENT) {
*_rules = NULL;
*_count = 0;
ret = EOK;
goto done;
} else if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Error looking up SUDO rules\n");
goto done;
}
This is how it's supposed to be :-) Thanks.