Fix issues found by LLVM checker.
authorAlan T. DeKok <aland@freeradius.org>
Fri, 8 May 2009 10:53:02 +0000 (12:53 +0200)
committerAlan T. DeKok <aland@freeradius.org>
Fri, 8 May 2009 10:53:02 +0000 (12:53 +0200)
These are mostly dead stores, etc.

26 files changed:
Make.inc.in
src/lib/Makefile
src/lib/valuepair.c
src/main/Makefile.in
src/main/evaluate.c
src/main/modcall.c
src/main/modules.c
src/main/radmin.c
src/main/realms.c
src/modules/Makefile
src/modules/rlm_acct_unique/rlm_acct_unique.c
src/modules/rlm_attr_rewrite/rlm_attr_rewrite.c
src/modules/rlm_detail/rlm_detail.c
src/modules/rlm_expr/paircmp.c
src/modules/rlm_expr/rlm_expr.c
src/modules/rlm_krb5/rlm_krb5.c
src/modules/rlm_mschap/rlm_mschap.c
src/modules/rlm_pap/rlm_pap.c
src/modules/rlm_policy/parse.c
src/modules/rlm_preprocess/rlm_preprocess.c
src/modules/rlm_radutmp/rlm_radutmp.c
src/modules/rlm_realm/rlm_realm.c
src/modules/rlm_sql/rlm_sql.c
src/modules/rlm_sql_log/rlm_sql_log.c
src/modules/rlm_unix/rlm_unix.c
src/modules/rules.mak

index 73c4b2d..771231f 100644 (file)
@@ -98,4 +98,11 @@ endif
 endif
 
 #  http://clang.llvm.org/StaticAnalysis.html
+#
+#  $ make scan | sed "s/.*Run '//;s/'.*//" > scan.sh
+#  $ ./scan.sh
+#
 SCAN_BUILD     = /path/to/checker-0.167/scan-build
+ifeq ($(SCAN),yes)
+CC             := $(SCAN_BUILD) gcc -DFR_SCAN_BUILD
+endif
index d9fae86..fb13f9c 100644 (file)
@@ -58,4 +58,4 @@ install: all
 
 .PHONY: scan
 scan:
-       @[ "$(SCAN_BUILD)" == "" ] || $(SCAN_BUILD) gcc -DFR_SCAN_BUILD $(CFLAGS) -c $(SRCS)
+       @[ "$(SCAN_BUILD)" == "" ] || ($(MAKE) SCAN=yes LIBTOOL= 2>&1) | grep 'scan-view' || true
index daf78bd..99c4119 100644 (file)
@@ -821,6 +821,15 @@ static const char *hextab = "0123456789abcdef";
  *  double-check the parsed value, to be sure it's legal for that
  *  type (length, etc.)
  */
+static uint32_t getint(const char *value, char **end)
+{
+       if ((value[0] == '0') && (value[1] == 'x')) {
+               return strtoul(value, end, 16);
+       }
+
+       return strtoul(value, end, 10);
+}
+
 VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
 {
        char            *p, *s=0;
@@ -944,7 +953,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                        /*
                         *      Note that ALL integers are unsigned!
                         */
-                       vp->vp_integer = (uint32_t) strtoul(value, &p, 10);
+                       vp->vp_integer = getint(value, &p);
                        if (!*p) {
                                if (vp->vp_integer > 255) {
                                        fr_strerror_printf("Byte value \"%s\" is larger than 255", value);
@@ -971,7 +980,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                        /*
                         *      Note that ALL integers are unsigned!
                         */
-                       vp->vp_integer = (uint32_t) strtoul(value, &p, 10);
+                       vp->vp_integer = getint(value, &p);
                        if (!*p) {
                                if (vp->vp_integer > 65535) {
                                        fr_strerror_printf("Byte value \"%s\" is larger than 65535", value);
@@ -998,7 +1007,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                        /*
                         *      Note that ALL integers are unsigned!
                         */
-                       vp->vp_integer = (uint32_t) strtoul(value, &p, 10);
+                       vp->vp_integer = getint(value, &p);
                        if (!*p) {
                                vp->length = 4;
                                break;
@@ -1259,7 +1268,7 @@ static VALUE_PAIR *pairmake_any(const char *attribute, const char *value,
                return NULL;
        }
 
-       attr = vendor = 0;
+       vendor = 0;
 
        /*
         *      Pull off vendor prefix first.
@@ -1880,6 +1889,8 @@ VALUE_PAIR *readvp2(FILE *fp, int *pfiledone, const char *errprefix)
  *     e.g. "foo" != "bar"
  *
  *     Returns true (comparison is true), or false (comparison is not true);
+ *
+ *     FIXME: Ignores tags!
  */
 int paircmp(VALUE_PAIR *one, VALUE_PAIR *two)
 {
index ac11464..6b47cd0 100644 (file)
@@ -151,4 +151,4 @@ endif
 
 .PHONY: scan
 scan:
-       @[ "$(SCAN_BUILD)" == "" ] || $(SCAN_BUILD) gcc -DFR_SCAN_BUILD $(CFLAGS) -c $(SERVER_SRCS)
+       @[ "$(SCAN_BUILD)" == "" ] || ($(MAKE) SCAN=yes LIBTOOL= 2>&1) | grep 'scan-view' || true
index 35067b4..bad147d 100644 (file)
@@ -1155,6 +1155,9 @@ void radius_pairmove(REQUEST *request, VALUE_PAIR **to, VALUE_PAIR *from)
                last = &(*last)->next;
        }
 
+       rad_assert(request != NULL);
+       rad_assert(request->packet != NULL);
+
        /*
         *      Fix dumb cache issues
         */
index cdc8732..54e8027 100644 (file)
@@ -267,10 +267,9 @@ static void safe_unlock(module_instance_t *instance)
 #define safe_unlock(foo)
 #endif
 
-static int call_modsingle(int component, modsingle *sp, REQUEST *request,
-                         int default_result)
+static int call_modsingle(int component, modsingle *sp, REQUEST *request)
 {
-       int myresult = default_result;
+       int myresult;
 
        rad_assert(request != NULL);
 
@@ -449,6 +448,15 @@ int modcall(int component, modcallable *c, REQUEST *request)
                                                       g->vps, child->name);
                        if (rcode != RLM_MODULE_UPDATED) {
                                myresult = rcode;
+                       } else {
+                               /*
+                                *      FIXME: Set priority based on
+                                *      previous priority, so that we
+                                *      don't stop on reject when the
+                                *      default priority was to
+                                *      continue...
+                                *      
+                                */
                        }
                        goto handle_result;
                }
@@ -627,8 +635,7 @@ int modcall(int component, modcallable *c, REQUEST *request)
                 */
                sp = mod_callabletosingle(child);
 
-               myresult = call_modsingle(child->method, sp, request,
-                                         default_component_results[component]);
+               myresult = call_modsingle(child->method, sp, request);
        handle_result:
                RDEBUG2("%.*s[%s] returns %s",
                       stack.pointer + 1, modcall_spaces,
index 47d2802..5ea8674 100644 (file)
@@ -455,7 +455,6 @@ module_instance_t *find_module_instance(CONF_SECTION *modules,
        if (!do_link) return NULL;
 
        name1 = cf_section_name1(cs);
-       name2 = cf_section_name2(cs);
 
        /*
         *      Found the configuration entry.
index c1520a1..9e2e596 100644 (file)
@@ -184,7 +184,6 @@ static ssize_t run_command(int sockfd, const char *command,
 {
        char *p;
        ssize_t size, len;
-       int flag = 1;
 
        if (echo) {
                fprintf(outputfp, "%s\n", command);
@@ -204,7 +203,7 @@ static ssize_t run_command(int sockfd, const char *command,
 
        memset(buffer, 0, bufsize);
 
-       while (flag == 1) {
+       while (1) {
                int rcode;
                fd_set readfds;
 
@@ -257,8 +256,6 @@ static ssize_t run_command(int sockfd, const char *command,
                        *p = '\0';
 
                        if (p[-1] == '\n') p[-1] = '\0';
-
-                       flag = 0;
                        break;
                }
        }
index 547f8ac..abccdbe 100644 (file)
@@ -1766,8 +1766,6 @@ home_server *home_server_ldb(const char *realmname,
        home_server     *found = NULL;
        VALUE_PAIR      *vp;
 
-       start = 0;
-
        /*
         *      Determine how to pick choose the home server.
         */
index 771cd14..63b4b5f 100644 (file)
@@ -50,3 +50,7 @@ common:
                        $(MAKE) $(MFLAGS) -C $$mod $(WHAT_TO_MAKE) || exit $$?; \
                fi; \
        done
+
+.PHONY: scan
+scan:
+       @[ "$(SCAN_BUILD)" == "" ] || ($(MAKE) SCAN=yes LIBTOOL= WHAT_TO_MAKE=scan common 2>&1) | grep 'scan-build' || true
index c2f0524..3eb4205 100644 (file)
@@ -84,8 +84,7 @@ static int unique_parse_key(rlm_acct_unique_t *inst, char *key)
        }
        *ptr = '\0';
 
-
-       keyptr = ptr = key;
+       ptr = key;
        while(ptr) {
                switch(*ptr) {
                case ',':
@@ -188,7 +187,6 @@ static int add_unique_id(void *instance, REQUEST *request)
        /* initialize variables */
        p = buffer;
        left = BUFFERLEN;
-       length = 0;
        cur = inst->head;
 
        /*
index 054ead8..de7f528 100644 (file)
@@ -168,7 +168,6 @@ static int do_attr_rewrite(void *instance, REQUEST *request)
        char *ptr, *ptr2;
        char search_STR[MAX_STRING_LEN];
        char replace_STR[MAX_STRING_LEN];
-       int replace_len = 0;
 
        if ((attr_vp = pairfind(request->config_items, PW_REWRITE_RULE)) != NULL){
                if (data->name == NULL || strcmp(data->name,attr_vp->vp_strvalue))
@@ -181,7 +180,6 @@ static int do_attr_rewrite(void *instance, REQUEST *request)
                        DEBUG2("%s: xlat on replace string failed.", data->name);
                        return ret;
                }
-               replace_len = strlen(replace_STR);
                attr_vp = pairmake(data->attribute,replace_STR,0);
                if (attr_vp == NULL){
                        DEBUG2("%s: Could not add new attribute %s with value '%s'", data->name,
@@ -221,6 +219,8 @@ static int do_attr_rewrite(void *instance, REQUEST *request)
                DEBUG2("%s: Added attribute %s with value '%s'", data->name,data->attribute,replace_STR);
                ret = RLM_MODULE_OK;
        } else {
+               int replace_len = 0;
+
                /* new_attribute = no */
                switch (data->searchin) {
                        case RLM_REGEX_INPACKET:
index 936a5c0..01ebb7d 100644 (file)
@@ -203,6 +203,8 @@ static int do_detail(void *instance, REQUEST *request, RADIUS_PACKET *packet,
 
        struct detail_instance *inst = instance;
 
+       rad_assert(request != NULL);
+
        /*
         *      Nothing to log: don't do anything.
         */
index 092fc51..944a62b 100644 (file)
@@ -157,7 +157,8 @@ static int presufcmp(UNUSED void *instance,
                 */
                vp = radius_paircreate(req, &request,
                                       PW_STRIPPED_USER_NAME, PW_TYPE_STRING);
-               if (vp) req->username = vp;
+               if (!vp) return ret;
+               req->username = vp;
        }
 
        strlcpy((char *)vp->vp_strvalue, rest, sizeof(vp->vp_strvalue));
@@ -219,7 +220,7 @@ static int genericcmp(void *instance UNUSED,
 
                snprintf(name, sizeof(name), "%%{%s}", check->name);
 
-               rcode = radius_xlat(value, sizeof(value), name, req, NULL);
+               radius_xlat(value, sizeof(value), name, req, NULL);
                vp = pairmake(check->name, value, check->operator);
 
                /*
index 6d19fcc..6b0456b 100644 (file)
@@ -171,10 +171,18 @@ static int get_number(REQUEST *request, const char **string, int *answer)
                        break;
 
                case TOKEN_DIVIDE:
+                       if (x == 0) {
+                               result = 0; /* we don't have NaN for integers */
+                               break;
+                       }
                        result /= x;
                        break;
 
                case TOKEN_REMAINDER:
+                       if (x == 0) {
+                               result = 0; /* we don't have NaN for integers */
+                               break;
+                       }
                        result %= x;
                        break;
 
index a5ed511..16fba41 100644 (file)
@@ -54,7 +54,7 @@ static int verify_krb5_tgt(krb5_context context, rlm_krb5_t *instance,
        char phost[BUFSIZ];
        krb5_principal princ;
        krb5_keyblock *keyblock = 0;
-       krb5_data packet;
+       krb5_data packet, *server;
        krb5_auth_context auth_context = NULL;
        krb5_keytab keytab;
        /* arbitrary 64-byte limit on service names; I've never seen a
@@ -86,7 +86,13 @@ static int verify_krb5_tgt(krb5_context context, rlm_krb5_t *instance,
                return RLM_MODULE_REJECT;
        }
 
-       strlcpy(phost, krb5_princ_component(c, princ, 1)->data, BUFSIZ);
+       server = krb5_princ_component(c, princ, 1);
+       if (!server) {
+               radlog(L_DBB, "rlm_krb5: [%s] krb5_princ_component failed.",
+                      user);
+               return RLM_MODULE_REJECT;
+       }
+       strlcpy(phost, server->data, BUFSIZ);
        phost[BUFSIZ - 1] = '\0';
 
        /*
index fd710f6..aed4133 100644 (file)
@@ -288,7 +288,7 @@ static size_t mschap_xlat(void *instance, REQUEST *request,
        VALUE_PAIR      *chap_challenge, *response;
        rlm_mschap_t    *inst = instance;
 
-       chap_challenge = response = NULL;
+       response = NULL;
 
        func = func;            /* -Wunused */
 
index 71b768b..540aa4d 100644 (file)
@@ -200,7 +200,7 @@ static int decode_it(const char *src, uint8_t *dst)
 
        dst[2] = (unsigned char)(x & 255); x >>= 8;
        dst[1] = (unsigned char)(x & 255); x >>= 8;
-       dst[0] = (unsigned char)(x & 255); x >>= 8;
+       dst[0] = (unsigned char)(x & 255);
 
        return 1;
 }
index ae201ab..a79830d 100644 (file)
@@ -141,8 +141,7 @@ static const char *policy_lex_string(const char *input,
                                     char *buffer, size_t buflen)
 {
        rad_assert(input != NULL);
-
-       if (buffer) *buffer = '\0';
+       rad_assert(buffer != NULL);
 
        switch (*input) {
        case '\0':
@@ -907,6 +906,7 @@ static int parse_if(policy_lex_file_t *lexer, policy_item_t **tail)
                          POLICY_RESERVED_UNKNOWN) == POLICY_RESERVED_ELSE)) {
                debug_tokens("[ELSE] ");
                token = policy_lex_file(lexer, 0, mystring, sizeof(mystring));
+               rad_assert(token == POLICY_LEX_BARE_WORD);
 
                token = policy_lex_file(lexer, POLICY_LEX_FLAG_PEEK,
                                        mystring, sizeof(mystring));
@@ -915,6 +915,7 @@ static int parse_if(policy_lex_file_t *lexer, policy_item_t **tail)
                                  POLICY_RESERVED_UNKNOWN) == POLICY_RESERVED_IF)) {
                        token = policy_lex_file(lexer, 0,
                                                mystring, sizeof(mystring));
+                       rad_assert(token == POLICY_LEX_BARE_WORD);
                        rcode = parse_if(lexer, &(this->if_false));
                } else {
                        rcode = parse_block(lexer, &(this->if_false));
@@ -1394,7 +1395,6 @@ static int parse_block(policy_lex_file_t *lexer, policy_item_t **tail)
                return 0;
        }
 
-       rcode = 0;
        while ((rcode = parse_statement(lexer, tail)) != 0) {
                if (rcode == 2) {
                        token = policy_lex_file(lexer, 0, NULL, 0);
index ca59a5f..5c25569 100644 (file)
@@ -626,7 +626,7 @@ static int preprocess_preaccounting(void *instance, REQUEST *request)
                return RLM_MODULE_FAIL;
        }
 
-       r = hints_setup(data->hints, request);
+       hints_setup(data->hints, request);
 
        if ((r = huntgroup_access(request,
                                  data->huntgroups)) != RLM_MODULE_OK) {
index 82fa13b..88e2a4a 100644 (file)
@@ -183,13 +183,10 @@ static int radutmp_accounting(void *instance, REQUEST *request)
        struct radutmp  ut, u;
        VALUE_PAIR      *vp;
        int             status = -1;
-       uint32_t        framed_address = 0;
        int             protocol = -1;
        time_t          t;
        int             fd;
-       int             just_an_update = 0;
        int             port_seen = 0;
-       int             nas_port_type = 0;
        int             off;
        rlm_radutmp_t   *inst = instance;
        char            buffer[256];
@@ -264,7 +261,6 @@ static int radutmp_accounting(void *instance, REQUEST *request)
                switch (vp->attribute) {
                        case PW_LOGIN_IP_HOST:
                        case PW_FRAMED_IP_ADDRESS:
-                               framed_address = vp->vp_ipaddr;
                                ut.framed_address = vp->vp_ipaddr;
                                break;
                        case PW_FRAMED_PROTOCOL:
@@ -301,7 +297,6 @@ static int radutmp_accounting(void *instance, REQUEST *request)
                        case PW_NAS_PORT_TYPE:
                                if (vp->vp_integer <= 4)
                                        ut.porttype = porttypes[vp->vp_integer];
-                               nas_port_type = vp->vp_integer;
                                break;
                        case PW_CALLING_STATION_ID:
                                if(inst->callerid_ok)
@@ -496,8 +491,6 @@ static int radutmp_accounting(void *instance, REQUEST *request)
                         *      Keep the original login time.
                         */
                        ut.time = u.time;
-                       if (u.login[0] != 0)
-                               just_an_update = 1;
                }
 
                if (lseek(fd, -(off_t)sizeof(u), SEEK_CUR) < 0) {
@@ -546,7 +539,6 @@ static int radutmp_accounting(void *instance, REQUEST *request)
                } else if (r == 0) {
                        radlog(L_ERR, "rlm_radutmp: Logout for NAS %s port %u, but no Login record",
                               nas, ut.nas_port);
-                       r = -1;
                }
        }
        close(fd);      /* and implicitely release the locks */
index 1ca475e..a893267 100644 (file)
@@ -91,7 +91,7 @@ static int check_for_realm(void *instance, REQUEST *request, REALM **returnrealm
         *      it already ( via another rlm_realm instance ) and should return.
         */
 
-       if ( (vp = pairfind(request->packet->vps, PW_REALM)) != NULL ) {
+       if (pairfind(request->packet->vps, PW_REALM) != NULL ) {
                RDEBUG2("Request already proxied.  Ignoring.");
                return RLM_MODULE_OK;
        }
index 0351378..4038ed1 100644 (file)
@@ -560,6 +560,7 @@ static int sql_get_grouplist (SQL_INST *inst, SQLSOCK *sqlsocket, REQUEST *reque
                        *group_list = rad_malloc(sizeof(SQL_GROUPLIST));
                        group_list_tmp = *group_list;
                } else {
+                       rad_assert(group_list_tmp != NULL);
                        group_list_tmp->next = rad_malloc(sizeof(SQL_GROUPLIST));
                        group_list_tmp = group_list_tmp->next;
                }
index d77e9b0..a1b0832 100644 (file)
@@ -204,6 +204,9 @@ static int sql_set_user(rlm_sql_log_t *inst, REQUEST *request, char *sqlusername
        tmpuser[0] = '\0';
        sqlusername[0] = '\0';
 
+       rad_assert(request != NULL);
+       rad_assert(request->packet != NULL);
+
        /* Remove any user attr we added previously */
        pairdelete(&request->packet->vps, PW_SQL_USER_NAME);
 
@@ -344,6 +347,9 @@ static int sql_log_accounting(void *instance, REQUEST *request)
        DICT_VALUE      *dval;
        CONF_PAIR       *cp;
 
+       rad_assert(request != NULL);
+       rad_assert(request->packet != NULL);
+
        RDEBUG("Processing sql_log_accounting");
 
        /* Find the Acct Status Type. */
@@ -383,6 +389,8 @@ static int sql_log_postauth(void *instance, REQUEST *request)
        char            querystr[MAX_QUERY_LEN];
        rlm_sql_log_t   *inst = (rlm_sql_log_t *)instance;
 
+       rad_assert(request != NULL);
+
        RDEBUG("Processing sql_log_postauth");
 
        /* Xlat the query */
index d97d52b..155a66c 100644 (file)
@@ -421,10 +421,11 @@ static int unix_accounting(void *instance, REQUEST *request)
        int             status = -1;
        int             nas_address = 0;
        int             framed_address = 0;
+#ifdef USER_PROCESS
        int             protocol = -1;
+#endif
        int             nas_port = 0;
        int             port_seen = 0;
-       int             nas_port_type = 0;
        struct unix_instance *inst = (struct unix_instance *) instance;
 
        /*
@@ -460,7 +461,7 @@ static int unix_accounting(void *instance, REQUEST *request)
         *      We're only interested in accounting messages
         *      with a username in it.
         */
-       if ((vp = pairfind(request->packet->vps, PW_USER_NAME)) == NULL)
+       if (pairfind(request->packet->vps, PW_USER_NAME) == NULL)
                return RLM_MODULE_NOOP;
 
        t = request->timestamp;
@@ -482,9 +483,11 @@ static int unix_accounting(void *instance, REQUEST *request)
                        case PW_FRAMED_IP_ADDRESS:
                                framed_address = vp->vp_ipaddr;
                                break;
+#ifdef USER_PROCESS
                        case PW_FRAMED_PROTOCOL:
                                protocol = vp->vp_integer;
                                break;
+#endif
                        case PW_NAS_IP_ADDRESS:
                                nas_address = vp->vp_ipaddr;
                                break;
@@ -495,9 +498,6 @@ static int unix_accounting(void *instance, REQUEST *request)
                        case PW_ACCT_DELAY_TIME:
                                delay = vp->vp_ipaddr;
                                break;
-                       case PW_NAS_PORT_TYPE:
-                               nas_port_type = vp->vp_ipaddr;
-                               break;
                }
        }
 
@@ -508,8 +508,6 @@ static int unix_accounting(void *instance, REQUEST *request)
        if (strncmp(ut.ut_name, "!root", sizeof(ut.ut_name)) == 0 || !port_seen)
                return RLM_MODULE_NOOP;
 
-       s = "";
-
        /*
         *      If we didn't find out the NAS address, use the
         *      originator's IP address.
index 90ffa7d..abf6083 100644 (file)
@@ -177,4 +177,4 @@ install:
 
 .PHONY: scan
 scan:
-       @[ "$(SCAN_BUILD)" == "" ] || $(SCAN_BUILD) gcc $(CFLAGS) $(RLM_CFLAGS) -c $(SRCS)
+       @[ "$(SCAN_BUILD)" == "" ] || ($(MAKE) SCAN=yes LIBTOOL= 2>&1) | grep 'scan-view' || true