From b048aacba5d279f3271163cfaa0704684beca1e2 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Tue, 5 Apr 2011 12:00:22 -0700 Subject: [PATCH] Add support for query_id, and fixes a few cases where programmer error can lead to memory issues. --- libmemcached/behavior.c | 16 +++++----- libmemcached/common.h | 27 +++++++++-------- libmemcached/delete.c | 48 +++++++++++++++++++++++++++-- libmemcached/dump.c | 11 ++++++- libmemcached/flush.c | 4 +++ libmemcached/get.c | 11 ++++--- libmemcached/include.am | 2 ++ libmemcached/memcached.c | 5 ++-- libmemcached/memcached.h | 3 +- libmemcached/quit.c | 23 +++++++++----- libmemcached/quit.h | 3 ++ libmemcached/server.c | 58 ++++++++++++++++++++++++++++++------ libmemcached/stats.c | 18 +++++++---- libmemcached/storage.c | 10 ++++--- support/libmemcached.spec.in | 3 ++ tests/mem_functions.c | 2 -- 16 files changed, 183 insertions(+), 61 deletions(-) diff --git a/libmemcached/behavior.c b/libmemcached/behavior.c index 8a9869f3..1480fdf5 100644 --- a/libmemcached/behavior.c +++ b/libmemcached/behavior.c @@ -55,7 +55,7 @@ memcached_return_t memcached_behavior_set(memcached_st *ptr, ptr->server_failure_limit= (uint32_t)data; break; case MEMCACHED_BEHAVIOR_BINARY_PROTOCOL: - memcached_quit(ptr); // We need t shutdown all of the connections to make sure we do the correct protocol + send_quit(ptr); // We need t shutdown all of the connections to make sure we do the correct protocol if (data) { ptr->flags.verify_key= false; @@ -67,11 +67,11 @@ memcached_return_t memcached_behavior_set(memcached_st *ptr, break; case MEMCACHED_BEHAVIOR_NO_BLOCK: ptr->flags.no_block= set_flag(data); - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_BUFFER_REQUESTS: ptr->flags.buffer_requests= set_flag(data); - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_USE_UDP: if (memcached_server_count(ptr)) @@ -86,11 +86,11 @@ memcached_return_t memcached_behavior_set(memcached_st *ptr, break; case MEMCACHED_BEHAVIOR_TCP_NODELAY: ptr->flags.tcp_nodelay= set_flag(data); - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_TCP_KEEPALIVE: ptr->flags.tcp_keepalive= set_flag(data); - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_DISTRIBUTION: return memcached_behavior_set_distribution(ptr, (memcached_server_distribution_t)data); @@ -144,15 +144,15 @@ memcached_return_t memcached_behavior_set(memcached_st *ptr, break; case MEMCACHED_BEHAVIOR_SOCKET_SEND_SIZE: ptr->send_size= (int32_t)data; - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_SOCKET_RECV_SIZE: ptr->recv_size= (int32_t)data; - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_TCP_KEEPIDLE: ptr->tcp_keepidle= (uint32_t)data; - memcached_quit(ptr); + send_quit(ptr); break; case MEMCACHED_BEHAVIOR_USER_DATA: return memcached_set_error_string(ptr, MEMCACHED_DEPRECATED, diff --git a/libmemcached/common.h b/libmemcached/common.h index e350149b..d0a4ab4c 100644 --- a/libmemcached/common.h +++ b/libmemcached/common.h @@ -71,10 +71,10 @@ #define BUILDING_LIBMEMCACHED 1 -#include "libmemcached/memcached.h" -#include "libmemcached/watchpoint.h" -#include "libmemcached/is.h" -#include "libmemcached/prefix_key.h" +#include +#include +#include +#include typedef struct memcached_server_st * memcached_server_write_instance_st; @@ -90,15 +90,16 @@ memcached_return_t memcached_server_execute(memcached_st *ptr, /* These are private not to be installed headers */ -#include "libmemcached/io.h" -#include "libmemcached/do.h" -#include "libmemcached/internal.h" -#include "libmemcached/array.h" -#include "libmemcached/libmemcached_probes.h" -#include "libmemcached/memcached/protocol_binary.h" -#include "libmemcached/byteorder.h" -#include "libmemcached/response.h" -#include "libmemcached/prefix_key.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include /* string value */ struct memcached_continuum_item_st diff --git a/libmemcached/delete.c b/libmemcached/delete.c index 2f205cd6..9db350c1 100644 --- a/libmemcached/delete.c +++ b/libmemcached/delete.c @@ -1,5 +1,42 @@ -#include "common.h" -#include "memcached/protocol_binary.h" +/* vim:expandtab:shiftwidth=2:tabstop=2:smarttab: + * + * Libmemcached library + * + * Copyright (C) 2011 Data Differential, http://datadifferential.com/ + * Copyright (C) 2006-2009 Brian Aker All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * + * * The names of its contributors may not be used to endorse or + * promote products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include +#include memcached_return_t memcached_delete(memcached_st *ptr, const char *key, size_t key_length, time_t expiration) @@ -20,13 +57,18 @@ memcached_return_t memcached_delete_by_key(memcached_st *ptr, time_t expiration) { bool to_write; - memcached_return_t rc; char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; uint32_t server_key; memcached_server_write_instance_st instance; LIBMEMCACHED_MEMCACHED_DELETE_START(); + memcached_return_t rc; + if ((rc= initialize_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + rc= memcached_validate_key_length(key_length, ptr->flags.binary_protocol); unlikely (rc != MEMCACHED_SUCCESS) diff --git a/libmemcached/dump.c b/libmemcached/dump.c index 9ec2dab5..18c15974 100644 --- a/libmemcached/dump.c +++ b/libmemcached/dump.c @@ -90,7 +90,16 @@ error: memcached_return_t memcached_dump(memcached_st *ptr, memcached_dump_fn *callback, void *context, uint32_t number_of_callbacks) { - /* No support for Binary protocol yet */ + memcached_return_t rc; + if ((rc= initialize_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + + /* + No support for Binary protocol yet + @todo Fix this so that we just flush, switch to ascii, and then go back to binary. + */ if (ptr->flags.binary_protocol) return MEMCACHED_FAILURE; diff --git a/libmemcached/flush.c b/libmemcached/flush.c index 5aaffa56..8da6b5b5 100644 --- a/libmemcached/flush.c +++ b/libmemcached/flush.c @@ -8,6 +8,10 @@ static memcached_return_t memcached_flush_textual(memcached_st *ptr, memcached_return_t memcached_flush(memcached_st *ptr, time_t expiration) { memcached_return_t rc; + if ((rc= initialize_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } LIBMEMCACHED_MEMCACHED_FLUSH_START(); if (ptr->flags.binary_protocol) diff --git a/libmemcached/get.c b/libmemcached/get.c index 7cb5ec04..39f53373 100644 --- a/libmemcached/get.c +++ b/libmemcached/get.c @@ -150,6 +150,12 @@ static memcached_return_t memcached_mget_by_key_real(memcached_st *ptr, unsigned int master_server_key= (unsigned int)-1; /* 0 is a valid server id! */ bool is_master_key_set= false; + memcached_return_t rc; + if ((rc= initialize_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + unlikely (ptr->flags.use_udp) return MEMCACHED_NOT_SUPPORTED; @@ -158,9 +164,6 @@ static memcached_return_t memcached_mget_by_key_real(memcached_st *ptr, if (number_of_keys == 0) return MEMCACHED_NOTFOUND; - if (memcached_server_count(ptr) == 0) - return MEMCACHED_NO_SERVERS; - if (ptr->flags.verify_key && (memcached_key_test(keys, key_length, number_of_keys) == MEMCACHED_BAD_KEY_PROVIDED)) return MEMCACHED_BAD_KEY_PROVIDED; @@ -211,7 +214,7 @@ static memcached_return_t memcached_mget_by_key_real(memcached_st *ptr, If a server fails we warn about errors and start all over with sending keys to the server. */ - memcached_return_t rc= MEMCACHED_SUCCESS; + WATCHPOINT_ASSERT(rc == MEMCACHED_SUCCESS); size_t hosts_connected= 0; for (uint32_t x= 0; x < number_of_keys; x++) { diff --git a/libmemcached/include.am b/libmemcached/include.am index 42e251d6..035725a3 100644 --- a/libmemcached/include.am +++ b/libmemcached/include.am @@ -43,6 +43,7 @@ nobase_include_HEADERS+= \ libmemcached/flush_buffers.h \ libmemcached/get.h \ libmemcached/hash.h \ + libmemcached/initialize_query.h \ libmemcached/memcached.h \ libmemcached/memcached.hpp \ libmemcached/memcached/protocol_binary.h \ @@ -112,6 +113,7 @@ libmemcached_libmemcached_la_SOURCES+= \ libmemcached/get.c \ libmemcached/hash.c \ libmemcached/hosts.c \ + libmemcached/initialize_query.cc \ libmemcached/io.c \ libmemcached/key.c \ libmemcached/memcached.c \ diff --git a/libmemcached/memcached.c b/libmemcached/memcached.c index 3d2539eb..3715f6eb 100644 --- a/libmemcached/memcached.c +++ b/libmemcached/memcached.c @@ -52,7 +52,6 @@ static const memcached_st global_copy= { .no_block= false, .no_reply= false, .randomize_replica_read= false, - .reuse_memory= false, .support_cas= false, .tcp_nodelay= false, .use_sort_hosts= false, @@ -129,7 +128,7 @@ static inline bool _memcached_init(memcached_st *self) static void _free(memcached_st *ptr, bool release_st) { /* If we have anything open, lets close it now */ - memcached_quit(ptr); + send_quit(ptr); memcached_server_list_free(memcached_server_list(ptr)); memcached_result_free(&ptr->result); @@ -236,9 +235,11 @@ memcached_return_t memcached_reset(memcached_st *ptr) return MEMCACHED_INVALID_ARGUMENTS; bool stored_is_allocated= memcached_is_allocated(ptr); + uint64_t query_id= ptr->query_id; _free(ptr, false); memcached_create(ptr); memcached_set_allocated(ptr, stored_is_allocated); + ptr->query_id= query_id; if (ptr->configure.filename) { diff --git a/libmemcached/memcached.h b/libmemcached/memcached.h index 10ff26df..e3817892 100644 --- a/libmemcached/memcached.h +++ b/libmemcached/memcached.h @@ -101,10 +101,8 @@ struct memcached_st { bool no_block:1; // Don't block bool no_reply:1; bool randomize_replica_read:1; - bool reuse_memory:1; bool support_cas:1; bool tcp_nodelay:1; - bool use_cache_lookups:1; bool use_sort_hosts:1; bool use_udp:1; bool verify_key:1; @@ -129,6 +127,7 @@ struct memcached_st { int send_size; int recv_size; void *user_data; + uint64_t query_id; uint32_t number_of_replicas; hashkit_st distribution_hashkit; memcached_result_st result; diff --git a/libmemcached/quit.c b/libmemcached/quit.c index a8247000..6d72906c 100644 --- a/libmemcached/quit.c +++ b/libmemcached/quit.c @@ -83,16 +83,23 @@ void memcached_quit_server(memcached_server_st *ptr, bool io_death) } } -void memcached_quit(memcached_st *ptr) +void send_quit(memcached_st *ptr) { - if (memcached_server_count(ptr)) + for (uint32_t x= 0; x < memcached_server_count(ptr); x++) { - for (uint32_t x= 0; x < memcached_server_count(ptr); x++) - { - memcached_server_write_instance_st instance= - memcached_server_instance_fetch(ptr, x); + memcached_server_write_instance_st instance= + memcached_server_instance_fetch(ptr, x); - memcached_quit_server(instance, false); - } + memcached_quit_server(instance, false); + } +} + +void memcached_quit(memcached_st *ptr) +{ + if (initialize_query(ptr) != MEMCACHED_SUCCESS) + { + return; } + + send_quit(ptr); } diff --git a/libmemcached/quit.h b/libmemcached/quit.h index e252a1ea..e640020a 100644 --- a/libmemcached/quit.h +++ b/libmemcached/quit.h @@ -22,6 +22,9 @@ void memcached_quit(memcached_st *ptr); LIBMEMCACHED_LOCAL void memcached_quit_server(memcached_server_st *ptr, bool io_death); +LIBMEMCACHED_LOCAL +void send_quit(memcached_st *ptr); + #ifdef __cplusplus } #endif diff --git a/libmemcached/server.c b/libmemcached/server.c index b446d131..4fe676b0 100644 --- a/libmemcached/server.c +++ b/libmemcached/server.c @@ -168,6 +168,12 @@ memcached_return_t memcached_server_cursor(const memcached_st *ptr, void *context, uint32_t number_of_callbacks) { + memcached_return_t rc; + if ((rc= initialize_const_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + for (uint32_t x= 0; x < memcached_server_count(ptr); x++) { memcached_server_instance_st instance= @@ -215,14 +221,20 @@ memcached_server_instance_st memcached_server_by_key(const memcached_st *ptr, uint32_t server_key; memcached_server_instance_st instance; - *error= memcached_validate_key_length(key_length, - ptr->flags.binary_protocol); - unlikely (*error != MEMCACHED_SUCCESS) + memcached_return_t rc; + if ((rc= initialize_const_query(ptr)) != MEMCACHED_SUCCESS) + { + if (error) + *error= rc; + return NULL; + } - unlikely (memcached_server_count(ptr) == 0) + if ((rc= memcached_validate_key_length(key_length, ptr->flags.binary_protocol)) != MEMCACHED_SUCCESS) { - *error= MEMCACHED_NO_SERVERS; + if (error) + *error= rc; + return NULL; } @@ -239,14 +251,22 @@ memcached_server_instance_st memcached_server_by_key(const memcached_st *ptr, } -void memcached_server_error_reset(memcached_server_st *ptr) +void memcached_server_error_reset(memcached_server_st *self) { - ptr->cached_server_error[0]= 0; + WATCHPOINT_ASSERT(self); + if (! self) + return; + + self->cached_server_error[0]= 0; } -memcached_server_instance_st memcached_server_get_last_disconnect(const memcached_st *ptr) +memcached_server_instance_st memcached_server_get_last_disconnect(const memcached_st *self) { - return ptr->last_disconnected_server; + WATCHPOINT_ASSERT(self); + if (! self) + return 0; + + return self->last_disconnected_server; } void memcached_server_list_free(memcached_server_list_st self) @@ -276,26 +296,46 @@ void memcached_server_list_free(memcached_server_list_st self) uint32_t memcached_servers_set_count(memcached_server_st *servers, uint32_t count) { + WATCHPOINT_ASSERT(servers); + if (! servers) + return 0; + return servers->number_of_hosts= count; } uint32_t memcached_server_count(const memcached_st *self) { + WATCHPOINT_ASSERT(self); + if (! self) + return 0; + return self->number_of_hosts; } const char *memcached_server_name(memcached_server_instance_st self) { + WATCHPOINT_ASSERT(self); + if (! self) + return NULL; + return self->hostname; } in_port_t memcached_server_port(memcached_server_instance_st self) { + WATCHPOINT_ASSERT(self); + if (! self) + return 0; + return self->port; } uint32_t memcached_server_response_count(memcached_server_instance_st self) { + WATCHPOINT_ASSERT(self); + if (! self) + return 0; + return self->cursor_active; } diff --git a/libmemcached/stats.c b/libmemcached/stats.c index 83d342a1..45b530e4 100644 --- a/libmemcached/stats.c +++ b/libmemcached/stats.c @@ -405,15 +405,16 @@ memcached_stat_st *memcached_stat(memcached_st *self, char *args, memcached_retu memcached_return_t rc; memcached_stat_st *stats; - if (! self) + if ((rc= initialize_query(self)) != MEMCACHED_SUCCESS) { - WATCHPOINT_ASSERT(self); + if (error) + *error= rc; + return NULL; } WATCHPOINT_ASSERT(error); - unlikely (self->flags.use_udp) { if (error) @@ -432,6 +433,7 @@ memcached_stat_st *memcached_stat(memcached_st *self, char *args, memcached_retu return NULL; } + WATCHPOINT_ASSERT(rc == MEMCACHED_SUCCESS); rc= MEMCACHED_SUCCESS; for (uint32_t x= 0; x < memcached_server_count(self); x++) { @@ -467,7 +469,6 @@ memcached_stat_st *memcached_stat(memcached_st *self, char *args, memcached_retu memcached_return_t memcached_stat_servername(memcached_stat_st *memc_stat, char *args, const char *hostname, in_port_t port) { - memcached_return_t rc; memcached_st memc; memcached_st *memc_ptr; memcached_server_write_instance_st instance; @@ -475,10 +476,17 @@ memcached_return_t memcached_stat_servername(memcached_stat_st *memc_stat, char memset(memc_stat, 0, sizeof(memcached_stat_st)); memc_ptr= memcached_create(&memc); - WATCHPOINT_ASSERT(memc_ptr); + if (! memc_ptr) + return MEMCACHED_MEMORY_ALLOCATION_FAILURE; memcached_server_add(&memc, hostname, port); + memcached_return_t rc; + if ((rc= initialize_query(memc_ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + instance= memcached_server_instance_fetch(memc_ptr, 0); if (memc.flags.binary_protocol) diff --git a/libmemcached/storage.c b/libmemcached/storage.c index c31a473a..331c7fae 100644 --- a/libmemcached/storage.c +++ b/libmemcached/storage.c @@ -67,20 +67,22 @@ static inline memcached_return_t memcached_send(memcached_st *ptr, { bool to_write; size_t write_length; - memcached_return_t rc; char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; uint32_t server_key; memcached_server_write_instance_st instance; WATCHPOINT_ASSERT(!(value == NULL && value_length > 0)); + memcached_return_t rc; + if ((rc= initialize_query(ptr)) != MEMCACHED_SUCCESS) + { + return rc; + } + rc= memcached_validate_key_length(key_length, ptr->flags.binary_protocol); unlikely (rc != MEMCACHED_SUCCESS) return rc; - unlikely (memcached_server_count(ptr) == 0) - return MEMCACHED_NO_SERVERS; - if (ptr->flags.verify_key && (memcached_key_test((const char **)&key, &key_length, 1) == MEMCACHED_BAD_KEY_PROVIDED)) return MEMCACHED_BAD_KEY_PROVIDED; diff --git a/support/libmemcached.spec.in b/support/libmemcached.spec.in index 98f21a31..8d14926d 100644 --- a/support/libmemcached.spec.in +++ b/support/libmemcached.spec.in @@ -96,6 +96,7 @@ you will need to install %{name}-devel. %{_mandir}/man1/memdump.1.gz %{_mandir}/man1/memerror.1.gz %{_mandir}/man1/memflush.1.gz +%{_mandir}/man1/memaslap.1.gz %{_mandir}/man1/memrm.1.gz %{_mandir}/man1/memslap.1.gz %{_mandir}/man1/memstat.1.gz @@ -121,6 +122,8 @@ you will need to install %{name}-devel. %{_includedir}/libmemcached/callback.h %{_includedir}/libmemcached/configure.h %{_includedir}/libmemcached/constants.h +%{_includedir}/libhashkit/str_algorithm.h +%{_includedir}/libmemcached/memcached/vbucket.h %{_includedir}/libmemcached/delete.h %{_includedir}/libmemcached/dump.h %{_includedir}/libmemcached/error.h diff --git a/tests/mem_functions.c b/tests/mem_functions.c index 15daa39f..95c33cbc 100644 --- a/tests/mem_functions.c +++ b/tests/mem_functions.c @@ -319,8 +319,6 @@ static test_return_t clone_test(memcached_st *memc) { // Test all of the flags test_true(memc_clone->flags.no_block == memc->flags.no_block); test_true(memc_clone->flags.tcp_nodelay == memc->flags.tcp_nodelay); - test_true(memc_clone->flags.reuse_memory == memc->flags.reuse_memory); - test_true(memc_clone->flags.use_cache_lookups == memc->flags.use_cache_lookups); test_true(memc_clone->flags.support_cas == memc->flags.support_cas); test_true(memc_clone->flags.buffer_requests == memc->flags.buffer_requests); test_true(memc_clone->flags.use_sort_hosts == memc->flags.use_sort_hosts); -- 2.30.2