On Fri, Jun 07, 2013 at 07:03:25PM +0200, Lukas Slebodnik wrote:
On (07/06/13 14:38), Ondrej Kos wrote:
>Hi,
>
>Attached patch fixes the following issue:
>https://fedorahosted.org/sssd/ticket/1971
>
>This is coverity bug, CIDs: 11851,11852,11853
>
>I tested this with NSCD configuration file malformed in various ways.
>
>The first thing is, NSCD won't even start, when nscd.conf doesn't
>meet the specifications. This means that with malformed configuration
>it can't even interfere with SSSD. On the other hand, the malformed
>configuration might go through the check as OK (depending on selected
>cachings), but since it won't run, it doesn't affect SSSD (and
>checking for this in the SSSD would mean to adopt check from nscd
>sources to be sure).
>
>Ondra
>--
>Ondrej Kos
>Associate Software Engineer
>Identity Management - SSSD
>Red Hat Czech
>From f3036c2dab6acec3840f4c8274cce5c87bec3061 Mon Sep 17 00:00:00 2001
>From: Ondrej Kos <okos(a)redhat.com>
>Date: Fri, 7 Jun 2013 14:20:29 +0200
>Subject: [PATCH] Don't test for NULL in nscd config check
>
>https://fedorahosted.org/sssd/ticket/1971
>
>Coverity IDs: 11851, 11852, 11853
>
>The NULL check on "entry" "service" and "enable" line
string parts is
>not necessary and triggers warnings in coverity scans.
>---
> src/util/nscd.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
>diff --git a/src/util/nscd.c b/src/util/nscd.c
>index
2a06394d3b5a78ab65dcf8f40746c372488f6c05..b6c25128e52b43bef4fb52b23edbd33898853288 100644
>--- a/src/util/nscd.c
>+++ b/src/util/nscd.c
>@@ -191,15 +191,10 @@ errno_t sss_nscd_parse_conf(const char *conf_path)
> }
> *pad = '\0';
>
>- if (entry != NULL &&
>- service != NULL &&
>- enabled != NULL) {
>+ if (!strcmp(entry, "enable-cache") &&
>+ !strcmp(enabled, "yes")) {
>
>- if (!strcmp(entry, "enable-cache") &&
>- !strcmp(enabled, "yes")) {
>-
>- occured |= sss_nscd_check_service(service);
>- }
>+ occured |= sss_nscd_check_service(service);
> }
> };
>
>--
>1.8.1.4
>
entry, service, enabled are "incremented version of" pointer line, which is
not NULL.
ACK
LS
Can we also remove the assignments of entry,service and enabled to NULL
now since they will always be non-NULL in the while() iteration?