summaryrefslogtreecommitdiff
path: root/common
diff options
context:
space:
mode:
authorShawn Routhier <sar@isc.org>2012-10-16 15:13:59 -0700
committerShawn Routhier <sar@isc.org>2012-10-16 15:13:59 -0700
commitf9472c0c71d5df8bfd2375db3634d6b99cba16e1 (patch)
tree6548600d666bf3011e2287ddb6eb7e67cb1e7730 /common
parent713cfecfe14842c7b95395cfd0b88871c2943f19 (diff)
downloadisc-dhcp-f9472c0c71d5df8bfd2375db3634d6b99cba16e1.tar.gz
-n [v4_2]
[rt23833] Clean up a number of items identified by the Coverity static analysis tool. Runs courtesy of Red Hat.
Diffstat (limited to 'common')
-rw-r--r--common/alloc.c11
-rw-r--r--common/comapi.c8
-rw-r--r--common/lpf.c7
-rw-r--r--common/options.c59
-rw-r--r--common/parse.c50
-rw-r--r--common/print.c14
-rw-r--r--common/tree.c38
7 files changed, 119 insertions, 68 deletions
diff --git a/common/alloc.c b/common/alloc.c
index f2ddc8ad..3cbd140a 100644
--- a/common/alloc.c
+++ b/common/alloc.c
@@ -3,7 +3,8 @@
Memory allocation... */
/*
- * Copyright (c) 2004-2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2007 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1996-2003 by Internet Software Consortium
*
* Permission to use, copy, modify, and distribute this software for any
@@ -831,11 +832,11 @@ int dns_host_entry_dereference (ptr, file, line)
#endif
}
- (*ptr) -> refcnt--;
- rc_register (file, line, ptr, *ptr, (*ptr) -> refcnt, 1, RC_MISC);
- if (!(*ptr) -> refcnt)
+ (*ptr)->refcnt--;
+ rc_register (file, line, ptr, *ptr, (*ptr)->refcnt, 1, RC_MISC);
+ if ((*ptr)->refcnt == 0) {
dfree ((*ptr), file, line);
- if ((*ptr) -> refcnt < 0) {
+ } else if ((*ptr)->refcnt < 0) {
log_error ("%s(%d): negative refcnt!", file, line);
#if defined (DEBUG_RC_HISTORY)
dump_rc_history (*ptr);
diff --git a/common/comapi.c b/common/comapi.c
index 90d2262c..a9ae043c 100644
--- a/common/comapi.c
+++ b/common/comapi.c
@@ -692,7 +692,6 @@ isc_result_t dhcp_subnet_signal_handler (omapi_object_t *h,
/* In this function h should be a (struct subnet *) */
isc_result_t status;
- int updatep = 0;
if (h -> type != dhcp_type_subnet)
return DHCP_R_INVALIDARG;
@@ -706,8 +705,7 @@ isc_result_t dhcp_subnet_signal_handler (omapi_object_t *h,
if (status == ISC_R_SUCCESS)
return status;
}
- if (updatep)
- return ISC_R_SUCCESS;
+
return ISC_R_NOTFOUND;
}
@@ -861,7 +859,6 @@ isc_result_t dhcp_shared_network_signal_handler (omapi_object_t *h,
/* In this function h should be a (struct shared_network *) */
isc_result_t status;
- int updatep = 0;
if (h -> type != dhcp_type_shared_network)
return DHCP_R_INVALIDARG;
@@ -875,8 +872,7 @@ isc_result_t dhcp_shared_network_signal_handler (omapi_object_t *h,
if (status == ISC_R_SUCCESS)
return status;
}
- if (updatep)
- return ISC_R_SUCCESS;
+
return ISC_R_NOTFOUND;
}
diff --git a/common/lpf.c b/common/lpf.c
index 16eecc95..44f5f542 100644
--- a/common/lpf.c
+++ b/common/lpf.c
@@ -4,7 +4,8 @@
Support Services in Vancouver, B.C. */
/*
- * Copyright (c) 2004,2007,2009 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2009,2012 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004,2007 by Internet Systems Consortium, Inc. ("ISC")
* Copyright (c) 1996-2003 by Internet Software Consortium
*
* Permission to use, copy, modify, and distribute this software for any
@@ -456,9 +457,9 @@ get_hw_addr(const char *name, struct hardware *hw) {
memcpy(&hw->hbuf[1], sa->sa_data, 6);
break;
case ARPHRD_FDDI:
- hw->hlen = 17;
+ hw->hlen = 7;
hw->hbuf[0] = HTYPE_FDDI;
- memcpy(&hw->hbuf[1], sa->sa_data, 16);
+ memcpy(&hw->hbuf[1], sa->sa_data, 6);
break;
default:
log_fatal("Unsupported device type %ld for \"%s\"",
diff --git a/common/options.c b/common/options.c
index 51940f16..cafb77f9 100644
--- a/common/options.c
+++ b/common/options.c
@@ -258,9 +258,14 @@ int parse_option_buffer (options, buffer, length, universe)
option_cache_reference(&op->next, nop, MDL);
option_cache_dereference(&nop, MDL);
} else {
- save_option_buffer(universe, options, bp,
- bp->data + offset, len,
- code, 1);
+ if (save_option_buffer(universe, options, bp,
+ bp->data + offset, len,
+ code, 1) == 0) {
+ log_error("parse_option_buffer: "
+ "save_option_buffer failed");
+ buffer_dereference(&bp, MDL);
+ return 0;
+ }
}
}
option_dereference(&option, MDL);
@@ -517,6 +522,8 @@ int fqdn_universe_decode (struct option_state *options,
* Load all options into a buffer, and then split them out into the three
* separate fields in the dhcp packet (options, file, and sname) where
* options can be stored.
+ *
+ * returns 0 on error, length of packet on success
*/
int
cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
@@ -553,10 +560,10 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
if (inpacket &&
(op = lookup_option(&dhcp_universe, inpacket->options,
- DHO_DHCP_MAX_MESSAGE_SIZE))) {
- evaluate_option_cache(&ds, inpacket,
- lease, client_state, in_options,
- cfg_options, scope, op, MDL);
+ DHO_DHCP_MAX_MESSAGE_SIZE)) &&
+ (evaluate_option_cache(&ds, inpacket, lease,
+ client_state, in_options,
+ cfg_options, scope, op, MDL) != 0)) {
if (ds.len >= sizeof (u_int16_t)) {
i = getUShort(ds.data);
if(!mms || (i < mms))
@@ -679,7 +686,7 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
* in the packet if there is space. Note that the option
* may only be included if the client supplied one.
*/
- if ((priority_len < PRIORITY_COUNT) &&
+ if ((inpacket != NULL) && (priority_len < PRIORITY_COUNT) &&
(lookup_option(&fqdn_universe, inpacket->options,
FQDN_ENCODED) != NULL))
priority_list[priority_len++] = DHO_FQDN;
@@ -695,7 +702,7 @@ cons_options(struct packet *inpacket, struct dhcp_packet *outpacket,
* DHCPINFORM or DHCPLEASEQUERY responses (if the client
* didn't request it).
*/
- if ((priority_len < PRIORITY_COUNT) &&
+ if ((inpacket != NULL) && (priority_len < PRIORITY_COUNT) &&
((inpacket->packet_type == DHCPDISCOVER) ||
(inpacket->packet_type == DHCPREQUEST)))
priority_list[priority_len++] = DHO_SUBNET_MASK;
@@ -1266,11 +1273,12 @@ store_options(int *ocount,
cfg_options,
vendor_cfg_option -> code);
if (tmp)
- evaluate_option_cache (&name, packet, lease,
- client_state,
- in_options,
- cfg_options,
- scope, tmp, MDL);
+ /* No need to check the return as we check name.len below */
+ (void) evaluate_option_cache (&name, packet, lease,
+ client_state,
+ in_options,
+ cfg_options,
+ scope, tmp, MDL);
} else if (vuname) {
name.data = (unsigned char *)s;
name.len = strlen (s);
@@ -1308,9 +1316,10 @@ store_options(int *ocount,
/* Find the value of the option... */
od.len = 0;
if (oc) {
- evaluate_option_cache (&od, packet,
- lease, client_state, in_options,
- cfg_options, scope, oc, MDL);
+ /* No need to check the return as we check od.len below */
+ (void) evaluate_option_cache (&od, packet,
+ lease, client_state, in_options,
+ cfg_options, scope, oc, MDL);
/* If we have encapsulation for this option, and an oc
* lookup succeeded, but the evaluation failed, it is
@@ -3143,9 +3152,11 @@ int fqdn_option_space_encapsulate (result, packet, lease, client_state,
struct option_cache *oc = (struct option_cache *)(ocp -> car);
if (oc -> option -> code > FQDN_SUBOPTION_COUNT)
continue;
- evaluate_option_cache (&results [oc -> option -> code],
- packet, lease, client_state, in_options,
- cfg_options, scope, oc, MDL);
+ /* No need to check the return code, we check the length later */
+ (void) evaluate_option_cache (&results[oc->option->code],
+ packet, lease, client_state,
+ in_options, cfg_options, scope,
+ oc, MDL);
}
/* We add a byte for the flags field.
* We add two bytes for the two RCODE fields.
@@ -3310,10 +3321,10 @@ fqdn6_option_space_encapsulate(struct data_string *result,
oc = (struct option_cache *)(ocp->car);
if (oc->option->code > FQDN_SUBOPTION_COUNT)
log_fatal("Impossible condition at %s:%d.", MDL);
-
- evaluate_option_cache(&results[oc->option->code], packet,
- lease, client_state, in_options,
- cfg_options, scope, oc, MDL);
+ /* No need to check the return code, we check the length later */
+ (void) evaluate_option_cache(&results[oc->option->code], packet,
+ lease, client_state, in_options,
+ cfg_options, scope, oc, MDL);
}
/* We add a byte for the flags field at the start of the option.
diff --git a/common/parse.c b/common/parse.c
index f72c0d65..1a4be3a7 100644
--- a/common/parse.c
+++ b/common/parse.c
@@ -675,7 +675,23 @@ void parse_lease_time (cfile, timep)
the token specified in separator. If max is zero, any number of
numbers will be parsed; otherwise, exactly max numbers are
expected. Base and size tell us how to internalize the numbers
- once they've been tokenized. */
+ once they've been tokenized.
+
+ buf - A pointer to space to return the parsed value, if it is null
+ then the function will allocate space for the return.
+
+ max - The maximum number of items to store. If zero there is no
+ maximum. When buf is null and the function needs to allocate space
+ it will do an allocation of max size at the beginning if max is non
+ zero. If max is zero then the allocation will be done later, after
+ the function has determined the size necessary for the incoming
+ string.
+
+ returns NULL on errors or a pointer to the value string on success.
+ The pointer will either be buf if it was non-NULL or newly allocated
+ space if buf was NULL
+ */
+
unsigned char *parse_numeric_aggregate (cfile, buf,
max, separator, base, size)
@@ -696,9 +712,8 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
bufp = (unsigned char *)dmalloc (*max * size / 8, MDL);
if (!bufp)
log_fatal ("no space for numeric aggregate");
- s = 0;
- } else
- s = bufp;
+ }
+ s = bufp;
do {
if (count) {
@@ -713,6 +728,9 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
parse_warn (cfile, "too few numbers.");
if (token != SEMI)
skip_to_semi (cfile);
+ /* free bufp if it was allocated */
+ if ((bufp != NULL) && (bufp != buf))
+ dfree(bufp, MDL);
return (unsigned char *)0;
}
token = next_token (&val, (unsigned *)0, cfile);
@@ -729,7 +747,17 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
(base != 16 || token != NUMBER_OR_NAME)) {
parse_warn (cfile, "expecting numeric value.");
skip_to_semi (cfile);
- return (unsigned char *)0;
+ /* free bufp if it was allocated */
+ if ((bufp != NULL) && (bufp != buf))
+ dfree(bufp, MDL);
+ /* free any linked numbers we may have allocated */
+ while (c) {
+ pair cdr = c->cdr;
+ dfree(c->car, MDL);
+ dfree(c, MDL);
+ c = cdr;
+ }
+ return (NULL);
}
/* If we can, convert the number now; otherwise, build
a linked list of all the numbers. */
@@ -747,6 +775,10 @@ unsigned char *parse_numeric_aggregate (cfile, buf,
/* If we had to cons up a list, convert it now. */
if (c) {
+ /*
+ * No need to cleanup bufp, to get here we didn't allocate
+ * bufp above
+ */
bufp = (unsigned char *)dmalloc (count * size / 8, MDL);
if (!bufp)
log_fatal ("no space for numeric aggregate.");
@@ -1870,7 +1902,7 @@ int parse_base64 (data, cfile)
parse_warn (cfile, "can't allocate buffer for base64 data.");
data -> len = 0;
data -> data = (unsigned char *)0;
- return 0;
+ goto out;
}
j = k = 0;
@@ -5258,6 +5290,8 @@ int parse_option_token (rv, cfile, fmt, expr, uniform, lookups)
return 0;
}
*fmt = g;
+ /* FALL THROUGH */
+ /* to get string value for the option */
case 'X':
token = peek_token (&val, (unsigned *)0, cfile);
if (token == NUMBER_OR_NAME || token == NUMBER) {
@@ -5549,6 +5583,8 @@ int parse_option_decl (oc, cfile)
"encapsulation format");
goto parse_exit;
}
+ /* FALL THROUGH */
+ /* to get string value for the option */
case 'X':
len = parse_X (cfile, &hunkbuf [hunkix],
sizeof hunkbuf - hunkix);
@@ -5760,8 +5796,6 @@ int parse_option_decl (oc, cfile)
bp = (struct buffer *)0;
if (!buffer_allocate (&bp, hunkix + nul_term, MDL))
log_fatal ("no memory to store option declaration.");
- if (!bp -> data)
- log_fatal ("out of memory allocating option data.");
memcpy (bp -> data, hunkbuf, hunkix + nul_term);
if (!option_cache_allocate (oc, MDL))
diff --git a/common/print.c b/common/print.c
index d5d0ae07..b113bec1 100644
--- a/common/print.c
+++ b/common/print.c
@@ -1134,6 +1134,7 @@ static unsigned print_subexpression (expr, buf, len)
buf [rv] = 0;
return rv;
}
+ break;
case expr_gethostname:
if (len > 13) {
@@ -1245,7 +1246,12 @@ int token_print_indent (FILE *file, int col, int indent,
const char *prefix,
const char *suffix, const char *buf)
{
- int len = strlen (buf) + strlen (prefix);
+ int len = 0;
+ if (prefix != NULL)
+ len = strlen (prefix);
+ if (buf != NULL)
+ len += strlen (buf);
+
if (col + len > 79) {
if (indent + len < 79) {
indent_spaces (file, indent);
@@ -1258,8 +1264,10 @@ int token_print_indent (FILE *file, int col, int indent,
fputs (prefix, file);
col += strlen (prefix);
}
- fputs (buf, file);
- col += len;
+ if ((buf != NULL) && (*buf != 0)) {
+ fputs (buf, file);
+ col += strlen(buf);
+ }
if (suffix && *suffix) {
if (col + strlen (suffix) > 79) {
indent_spaces (file, indent);
diff --git a/common/tree.c b/common/tree.c
index c82ac271..17db4c06 100644
--- a/common/tree.c
+++ b/common/tree.c
@@ -2902,10 +2902,17 @@ int evaluate_numeric_expression (result, packet, lease, client_state,
return 0;
}
-/* Return data hanging off of an option cache structure, or if there
- isn't any, evaluate the expression hanging off of it and return the
- result of that evaluation. There should never be both an expression
- and a valid data_string. */
+/*
+ * Return data hanging off of an option cache structure, or if there
+ * isn't any, evaluate the expression hanging off of it and return the
+ * result of that evaluation. There should never be both an expression
+ * and a valid data_string.
+ *
+ * returns 0 if there wasn't an expression or it couldn't be evaluated
+ * returns non-zero if there was an expression or string that was evaluated
+ * When it returns zero the arguements, in particualr resutl, should not
+ * be modified
+ */
int evaluate_option_cache (result, packet, lease, client_state,
in_options, cfg_options, scope, oc, file, line)
@@ -3613,6 +3620,7 @@ int write_expression (file, expr, col, indent, firstp)
col = write_expression (file, expr -> data.suffix.len,
col, scol, 0);
col = token_print_indent (file, col, indent, "", "", ")");
+ break;
case expr_lcase:
col = token_print_indent(file, col, indent, "", "", "lcase");
@@ -4148,19 +4156,10 @@ int fundef_dereference (ptr, file, line)
const char *file;
int line;
{
- struct fundef *bp = *ptr;
+ struct fundef *bp;
struct string_list *sp, *next;
- if (!ptr) {
- log_error ("%s(%d): null pointer", file, line);
-#if defined (POINTER_DEBUG)
- abort ();
-#else
- return 0;
-#endif
- }
-
- if (!bp) {
+ if ((ptr == NULL) || (*ptr == NULL)) {
log_error ("%s(%d): null pointer", file, line);
#if defined (POINTER_DEBUG)
abort ();
@@ -4169,6 +4168,7 @@ int fundef_dereference (ptr, file, line)
#endif
}
+ bp = *ptr;
bp -> refcnt--;
rc_register (file, line, ptr, bp, bp -> refcnt, 1, RC_MISC);
if (bp -> refcnt < 0) {
@@ -4440,11 +4440,11 @@ int find_bound_string (struct data_string *value,
if (binding -> value -> value.data.terminated) {
data_string_copy (value, &binding -> value -> value.data, MDL);
} else {
- buffer_allocate (&value -> buffer,
- binding -> value -> value.data.len,
- MDL);
- if (!value -> buffer)
+ if (buffer_allocate (&value->buffer,
+ binding->value->value.data.len,
+ MDL) == 0) {
return 0;
+ }
memcpy (value -> buffer -> data,
binding -> value -> value.data.data,