From d289ee683efb5245138d951bd9d934bcc196d550 Mon Sep 17 00:00:00 2001 From: Shawn Routhier Date: Tue, 10 Apr 2012 21:26:44 +0000 Subject: [PATCH] Remove unnecessary checks in the lease query code and clean up several compiler issues (some dereferences of NULL and treating an int as a boolean). [ISC-Bugs #26203] --- RELNOTES | 5 +++ common/execute.c | 79 ++++++++++++++++++++-------------------- server/bootp.c | 30 ++++++++------- server/confpars.c | 81 ++++++++++++++++++++--------------------- server/dhcpleasequery.c | 15 ++------ server/omapi.c | 27 +++++++------- 6 files changed, 118 insertions(+), 119 deletions(-) diff --git a/RELNOTES b/RELNOTES index 3b6c9b316..5d3149bed 100644 --- a/RELNOTES +++ b/RELNOTES @@ -124,6 +124,11 @@ work on other platforms. Please report any problems and suggested fixes to [ISC-Bugs #23138] [ISC-Bugs #27945] [ISC-Bugs #25586] [ISC-Bugs #27684] +- Remove unnecessary checks in the lease query code and clean up + several compiler issues (some dereferences of NULL and treating + an int as a boolean). + [ISC-Bugs #26203] + Changes since 4.2.2 - Fix the code that checks for an existing DDNS transaction to cancel diff --git a/common/execute.c b/common/execute.c index 9d08207a6..363ffa659 100644 --- a/common/execute.c +++ b/common/execute.c @@ -3,7 +3,8 @@ Support for executable statements. */ /* - * 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) 1998-2003 by Internet Software Consortium * * Permission to use, copy, modify, and distribute this software for any @@ -327,66 +328,66 @@ int execute_statements (result, packet, lease, client_state, case set_statement: case define_statement: if (!scope) { - log_error ("set %s: no scope", - r -> data.set.name); + log_error("set %s: no scope", + r->data.set.name); status = 0; break; } if (!*scope) { - if (!binding_scope_allocate (scope, MDL)) { - log_error ("set %s: can't allocate scope", - r -> data.set.name); + if (!binding_scope_allocate(scope, MDL)) { + log_error("set %s: can't allocate scope", + r->data.set.name); status = 0; break; } } - binding = find_binding (*scope, r -> data.set.name); + binding = find_binding(*scope, r->data.set.name); #if defined (DEBUG_EXPRESSIONS) - log_debug ("exec: set %s", r -> data.set.name); + log_debug("exec: set %s", r->data.set.name); #endif - if (!binding) { - binding = dmalloc (sizeof *binding, MDL); - if (binding) { - memset (binding, 0, sizeof *binding); - binding -> name = - dmalloc (strlen - (r -> data.set.name) + 1, - MDL); - if (binding -> name) { - strcpy (binding -> name, - r -> data.set.name); - binding -> next = (*scope) -> bindings; - (*scope) -> bindings = binding; + if (binding == NULL) { + binding = dmalloc(sizeof(*binding), MDL); + if (binding != NULL) { + memset(binding, 0, sizeof(*binding)); + binding->name = + dmalloc(strlen + (r->data.set.name) + 1, + MDL); + if (binding->name != NULL) { + strcpy(binding->name, r->data.set.name); + binding->next = (*scope)->bindings; + (*scope)->bindings = binding; } else { - dfree (binding, MDL); - binding = (struct binding *)0; + dfree(binding, MDL); + binding = NULL; } } } - if (binding) { - if (binding -> value) + if (binding != NULL) { + if (binding->value != NULL) binding_value_dereference - (&binding -> value, MDL); - if (r -> op == set_statement) { + (&binding->value, MDL); + if (r->op == set_statement) { status = (evaluate_expression - (&binding -> value, packet, + (&binding->value, packet, lease, client_state, in_options, out_options, - scope, r -> data.set.expr, + scope, r->data.set.expr, MDL)); } else { if (!(binding_value_allocate - (&binding -> value, MDL))) { - dfree (binding, MDL); - binding = (struct binding *)0; + (&binding->value, MDL))) { + dfree(binding, MDL); + binding = NULL; } - if (binding -> value) { - binding -> value -> type = - binding_function; - (fundef_reference - (&binding -> value -> value.fundef, - r -> data.set.expr -> data.func, - MDL)); + if ((binding != NULL) && + (binding->value != NULL)) { + binding->value->type = + binding_function; + (fundef_reference + (&binding->value->value.fundef, + r->data.set.expr->data.func, + MDL)); } } } diff --git a/server/bootp.c b/server/bootp.c index 477bb0457..f0b787c16 100644 --- a/server/bootp.c +++ b/server/bootp.c @@ -177,11 +177,12 @@ void bootp (packet) } /* Execute the host statements. */ - execute_statements_in_scope ((struct binding_value **)0, - packet, lease, (struct client_state *)0, - packet -> options, options, - &lease -> scope, - hp -> group, lease -> subnet -> group); + if (hp != NULL) { + execute_statements_in_scope (NULL, packet, lease, NULL, + packet->options, options, + &lease->scope, + hp->group, lease->subnet->group); + } /* Drop the request if it's not allowed for this client. */ if ((oc = lookup_option (&server_universe, options, SV_ALLOW_BOOTP)) && @@ -362,15 +363,16 @@ void bootp (packet) } /* Report what we're doing... */ - log_info ("%s", msgbuf); - log_info ("BOOTREPLY for %s to %s (%s) via %s", - piaddr (lease->ip_addr), hp -> name, - print_hw_addr (packet -> raw -> htype, - packet -> raw -> hlen, - packet -> raw -> chaddr), - packet -> raw -> giaddr.s_addr - ? inet_ntoa (packet -> raw -> giaddr) - : packet -> interface -> name); + log_info("%s", msgbuf); + log_info("BOOTREPLY for %s to %s (%s) via %s", + piaddr(lease->ip_addr), + ((hp != NULL) && (hp->name != NULL)) ? hp -> name : "unknown", + print_hw_addr (packet->raw->htype, + packet->raw->hlen, + packet->raw->chaddr), + packet->raw->giaddr.s_addr + ? inet_ntoa (packet->raw->giaddr) + : packet->interface->name); /* Set up the parts of the address that are in common. */ to.sin_family = AF_INET; diff --git a/server/confpars.c b/server/confpars.c index 1d822536f..64272b1b4 100644 --- a/server/confpars.c +++ b/server/confpars.c @@ -2731,77 +2731,76 @@ void parse_group_declaration (cfile, group) int dynamicp = 0; int staticp = 0; - g = (struct group *)0; - if (!clone_group (&g, group, MDL)) - log_fatal ("no memory for explicit group."); + g = NULL; + if (!clone_group(&g, group, MDL)) + log_fatal("no memory for explicit group."); - token = peek_token (&val, (unsigned *)0, cfile); + token = peek_token(&val, NULL, cfile); if (is_identifier (token) || token == STRING) { - next_token (&val, (unsigned *)0, cfile); + next_token(&val, NULL, cfile); - name = dmalloc (strlen (val) + 1, MDL); + name = dmalloc(strlen(val) + 1, MDL); if (!name) - log_fatal ("no memory for group decl name %s", val); - strcpy (name, val); + log_fatal("no memory for group decl name %s", val); + strcpy(name, val); } - if (!parse_lbrace (cfile)) { - group_dereference (&g, MDL); + if (!parse_lbrace(cfile)) { + group_dereference(&g, MDL); return; } do { - token = peek_token (&val, (unsigned *)0, cfile); + token = peek_token(&val, NULL, cfile); if (token == RBRACE) { - token = next_token (&val, (unsigned *)0, cfile); + token = next_token(&val, NULL, cfile); break; } else if (token == END_OF_FILE) { - token = next_token (&val, (unsigned *)0, cfile); - parse_warn (cfile, "unexpected end of file"); + token = next_token(&val, NULL, cfile); + parse_warn(cfile, "unexpected end of file"); break; } else if (token == TOKEN_DELETED) { - token = next_token (&val, (unsigned *)0, cfile); - parse_semi (cfile); + token = next_token(&val, NULL, cfile); + parse_semi(cfile); deletedp = 1; } else if (token == DYNAMIC) { - token = next_token (&val, (unsigned *)0, cfile); - parse_semi (cfile); + token = next_token(&val, NULL, cfile); + parse_semi(cfile); dynamicp = 1; } else if (token == STATIC) { - token = next_token (&val, (unsigned *)0, cfile); - parse_semi (cfile); + token = next_token(&val, NULL, cfile); + parse_semi(cfile); staticp = 1; } - declaration = parse_statement (cfile, g, GROUP_DECL, - (struct host_decl *)0, - declaration); + declaration = parse_statement(cfile, g, GROUP_DECL, + NULL, declaration); } while (1); if (name) { if (deletedp) { if (group_name_hash) { - t = (struct group_object *)0; - if (group_hash_lookup (&t, group_name_hash, - name, - strlen (name), MDL)) { - delete_group (t, 0); + t = NULL; + if (group_hash_lookup(&t, group_name_hash, + name, + strlen(name), MDL)) { + delete_group(t, 0); } } } else { - t = (struct group_object *)0; - status = group_object_allocate (&t, MDL); + t = NULL; + status = group_object_allocate(&t, MDL); if (status != ISC_R_SUCCESS) - log_fatal ("no memory for group decl %s: %s", - val, isc_result_totext (status)); - group_reference (&t -> group, g, MDL); - t -> name = name; - t -> flags = ((staticp ? GROUP_OBJECT_STATIC : 0) | - (dynamicp ? GROUP_OBJECT_DYNAMIC : 0) | - (deletedp ? GROUP_OBJECT_DELETED : 0)); - supersede_group (t, 0); - } - if (t) - group_object_dereference (&t, MDL); + log_fatal("no memory for group decl %s: %s", + val, isc_result_totext(status)); + group_reference(&t->group, g, MDL); + t->name = name; + t->flags = ((staticp ? GROUP_OBJECT_STATIC : 0) | + (dynamicp ? GROUP_OBJECT_DYNAMIC : 0) | + (deletedp ? GROUP_OBJECT_DELETED : 0)); + supersede_group(t, 0); + } + if (t != NULL) + group_object_dereference(&t, MDL); } } diff --git a/server/dhcpleasequery.c b/server/dhcpleasequery.c index 9daff8949..09913c24b 100644 --- a/server/dhcpleasequery.c +++ b/server/dhcpleasequery.c @@ -1,4 +1,5 @@ /* + * Copyright (C) 2011-2012 by Internet Systems Consortium, Inc. ("ISC") * Copyright (C) 2006-2007,2009 by Internet Systems Consortium, Inc. ("ISC") * * Permission to use, copy, modify, and distribute this software for any @@ -454,10 +455,7 @@ dhcpleasequery(struct packet *packet, int ms_nulltp) { (lease_duration / 8); if (time_renewal > cur_time) { - if (time_renewal < cur_time) - time_renewal = 0; - else - time_renewal = htonl(time_renewal - cur_time); + time_renewal = htonl(time_renewal - cur_time); if (!add_option(options, DHO_DHCP_RENEWAL_TIME, @@ -487,15 +485,8 @@ dhcpleasequery(struct packet *packet, int ms_nulltp) { } if (lease->ends > cur_time) { - if (time_expiry < cur_time) { - log_error("Impossible condition at %s:%d.", - MDL); - - option_state_dereference(&options, MDL); - lease_dereference(&lease, MDL); - return; - } time_expiry = htonl(lease->ends - cur_time); + if (!add_option(options, DHO_DHCP_LEASE_TIME, &time_expiry, diff --git a/server/omapi.c b/server/omapi.c index 22b5358aa..19b3847cc 100644 --- a/server/omapi.c +++ b/server/omapi.c @@ -984,20 +984,21 @@ isc_result_t dhcp_host_set_value (omapi_object_t *h, if (!omapi_ds_strcmp (name, "hardware-type")) { int type; - if (value && (value -> type == omapi_datatype_data && - value -> u.buffer.len == sizeof type)) { - if (value -> u.buffer.len > sizeof type) - return DHCP_R_INVALIDARG; - memcpy (&type, - value -> u.buffer.value, - value -> u.buffer.len); - type = ntohl (type); - } else if (value -> type == omapi_datatype_int) - type = value -> u.integer; + if ((value != NULL) && + ((value->type == omapi_datatype_data) && + (value->u.buffer.len == sizeof(type)))) { + if (value->u.buffer.len > sizeof(type)) + return (DHCP_R_INVALIDARG); + memcpy(&type, value->u.buffer.value, + value->u.buffer.len); + type = ntohl(type); + } else if ((value != NULL) && + (value->type == omapi_datatype_int)) + type = value->u.integer; else - return DHCP_R_INVALIDARG; - host -> interface.hbuf [0] = type; - return ISC_R_SUCCESS; + return (DHCP_R_INVALIDARG); + host->interface.hbuf[0] = type; + return (ISC_R_SUCCESS); } if (!omapi_ds_strcmp (name, "dhcp-client-identifier")) { -- 2.39.5