From aabc9fc24b965a3ac14bce9fd033aaba012c494b Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 26 Dec 2011 23:39:58 -0800 Subject: [PATCH] Convert storage over to use vector better. --- clients/memcp.cc | 33 +++--- libmemcached/auto.cc | 2 +- libmemcached/delete.cc | 6 +- libmemcached/storage.cc | 135 +++++++++++------------- tests/libmemcached-1.0/mem_functions.cc | 101 +++++++++--------- 5 files changed, 133 insertions(+), 144 deletions(-) diff --git a/clients/memcp.cc b/clients/memcp.cc index 4f4f8e10..38462213 100644 --- a/clients/memcp.cc +++ b/clients/memcp.cc @@ -61,8 +61,8 @@ static long strtol_wrapper(const char *nptr, int base, bool *error) /* Check for various possible errors */ - if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) - || (errno != 0 && val == 0)) + if ((errno == ERANGE and (val == LONG_MAX or val == LONG_MIN)) + or (errno != 0 && val == 0)) { *error= true; return EXIT_SUCCESS; @@ -143,7 +143,7 @@ int main(int argc, char *argv[]) { if (opt_verbose) { - fprintf(stderr, "memcp: %s: %s\n", argv[optind], strerror(errno)); + std::cerr << "memcp " << argv[optind] << " " << strerror(errno) << std::endl; optind++; } exit_code= EXIT_FAILURE; @@ -175,45 +175,46 @@ int main(int argc, char *argv[]) char *file_buffer_ptr; if ((file_buffer_ptr= (char *)malloc(sizeof(char) * (size_t)sbuf.st_size)) == NULL) { - fprintf(stderr, "malloc: %s\n", strerror(errno)); + std::cerr << "Error allocating file buffer(" << strerror(errno) << ")" << std::endl; exit(EXIT_FAILURE); } ssize_t read_length; if ((read_length= read(fd, file_buffer_ptr, (size_t)sbuf.st_size)) == -1) { - fprintf(stderr, "read: %s\n", strerror(errno)); + std::cerr << "Error while reading file " << file_buffer_ptr << " (" << strerror(errno) << ")" << std::endl; exit(EXIT_FAILURE); } if (read_length != sbuf.st_size) { - fprintf(stderr, "Failure reading from file\n"); - exit(1); + std::cerr << "Failure while reading file. Read length was not equal to stat() length" << std::endl; + exit(EXIT_FAILURE); } memcached_return_t rc; if (opt_method == OPT_ADD) + { rc= memcached_add(memc, ptr, strlen(ptr), file_buffer_ptr, (size_t)sbuf.st_size, opt_expires, opt_flags); + } else if (opt_method == OPT_REPLACE) + { rc= memcached_replace(memc, ptr, strlen(ptr), file_buffer_ptr, (size_t)sbuf.st_size, opt_expires, opt_flags); + } else + { rc= memcached_set(memc, ptr, strlen(ptr), file_buffer_ptr, (size_t)sbuf.st_size, opt_expires, opt_flags); + } if (rc != MEMCACHED_SUCCESS) { - fprintf(stderr, "memcp: %s: memcache error %s", - ptr, memcached_strerror(memc, rc)); - if (memcached_last_error_errno(memc)) - fprintf(stderr, " system error %s", strerror(memcached_last_error_errno(memc))); - fprintf(stderr, "\n"); - + std::cerr << "Error occrrured during operation: " << memcached_last_error_message(memc) << std::endl; exit_code= EXIT_FAILURE; } @@ -225,9 +226,14 @@ int main(int argc, char *argv[]) memcached_free(memc); if (opt_servers) + { free(opt_servers); + } + if (opt_hash) + { free(opt_hash); + } return exit_code; } @@ -274,6 +280,7 @@ static void options_parse(int argc, char *argv[]) { case 0: break; + case OPT_BINARY: opt_binary= true; break; diff --git a/libmemcached/auto.cc b/libmemcached/auto.cc index 8879634f..01a0132c 100644 --- a/libmemcached/auto.cc +++ b/libmemcached/auto.cc @@ -60,7 +60,7 @@ static memcached_return_t text_incr_decr(memcached_st *ptr, instance= memcached_server_instance_fetch(ptr, server_key); int send_length= snprintf(buffer, sizeof(buffer), " %" PRIu64, offset); - if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) + if (size_t(send_length) >= sizeof(buffer) or send_length < 0) { return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); diff --git a/libmemcached/delete.cc b/libmemcached/delete.cc index f5a60e33..9846bb51 100644 --- a/libmemcached/delete.cc +++ b/libmemcached/delete.cc @@ -61,10 +61,10 @@ static inline memcached_return_t ascii_delete(memcached_st *ptr, { memcached_literal_param("\r\n") } }; - size_t send_length= io_vector_total_size(vector, 5); - - if (ptr->flags.use_udp and flush == false) + if (memcached_is_udp(instance->root)) { + size_t send_length= io_vector_total_size(vector, 5); + if (send_length > MAX_UDP_DATAGRAM_LENGTH - UDP_DATAGRAM_HEADER_LENGTH) { return MEMCACHED_WRITE_FAILURE; diff --git a/libmemcached/storage.cc b/libmemcached/storage.cc index 416b3347..a7f68f0f 100644 --- a/libmemcached/storage.cc +++ b/libmemcached/storage.cc @@ -239,72 +239,73 @@ static memcached_return_t memcached_send_binary(memcached_st *ptr, static memcached_return_t memcached_send_ascii(memcached_st *ptr, memcached_server_write_instance_st instance, const char *key, - size_t key_length, + const size_t key_length, const char *value, - size_t value_length, - time_t expiration, - uint32_t flags, - uint64_t cas, - bool flush, - memcached_storage_action_t verb) + const size_t value_length, + const time_t expiration, + const uint32_t flags, + const uint64_t cas, + const bool flush, + const memcached_storage_action_t verb) { - size_t write_length; - char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; + // Invert the logic to make it simpler to read the code + bool reply= (ptr->flags.no_reply) ? false : true; + char flags_buffer[MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH +1]; + int flags_buffer_length= snprintf(flags_buffer, sizeof(flags_buffer), " %u", flags); + if (size_t(flags_buffer_length) >= sizeof(flags_buffer) or flags_buffer_length < 0) + { + return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH)")); + } + + char expiration_buffer[MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH +1]; + int expiration_buffer_length= snprintf(expiration_buffer, sizeof(expiration_buffer), " %llu", (unsigned long long)expiration); + if (size_t(expiration_buffer_length) >= sizeof(expiration_buffer) or expiration_buffer_length < 0) + { + return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH)")); + } + + char value_buffer[MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH +1]; + int value_buffer_length= snprintf(value_buffer, sizeof(value_buffer), " %llu", (unsigned long long)value_length); + if (size_t(value_buffer_length) >= sizeof(value_buffer) or value_buffer_length < 0) + { + return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH)")); + } + + char cas_buffer[MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH +1]; + int cas_buffer_length= 0; if (cas) { - int check_length; - check_length= snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, - "%s %.*s%.*s %u %llu %lu %llu%s\r\n", - storage_op_string(verb), - memcached_print_array(ptr->_namespace), - (int)key_length, key, flags, - (unsigned long long)expiration, (unsigned long)value_length, - (unsigned long long)cas, - (ptr->flags.no_reply) ? " noreply" : ""); - if (check_length >= MEMCACHED_DEFAULT_COMMAND_SIZE or check_length < 0) + cas_buffer_length= snprintf(cas_buffer, sizeof(cas_buffer), " %llu", (unsigned long long)cas); + if (size_t(cas_buffer_length) >= sizeof(cas_buffer) or cas_buffer_length < 0) { return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, - memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); + memcached_literal_param("snprintf(MEMCACHED_MAXIMUM_INTEGER_DISPLAY_LENGTH)")); } - write_length= check_length; } - else - { - char *buffer_ptr= buffer; - const char *command= storage_op_string(verb); - - /* Copy in the command, no space needed, we handle that in the command function*/ - memcpy(buffer_ptr, command, strlen(command)); - - /* Copy in the key prefix, switch to the buffer_ptr */ - buffer_ptr= (char *)memcpy((char *)(buffer_ptr + strlen(command)), (char *)memcached_array_string(ptr->_namespace), memcached_array_size(ptr->_namespace)); - - /* Copy in the key, adjust point if a key prefix was used. */ - buffer_ptr= (char *)memcpy(buffer_ptr + memcached_array_size(ptr->_namespace), - key, key_length); - buffer_ptr+= key_length; - buffer_ptr[0]= ' '; - buffer_ptr++; - - write_length= (size_t)(buffer_ptr - buffer); - int check_length= snprintf(buffer_ptr, MEMCACHED_DEFAULT_COMMAND_SIZE -(size_t)(buffer_ptr - buffer), - "%u %llu %lu%s\r\n", - flags, - (unsigned long long)expiration, (unsigned long)value_length, - ptr->flags.no_reply ? " noreply" : ""); - if ((size_t)check_length >= MEMCACHED_DEFAULT_COMMAND_SIZE -size_t(buffer_ptr - buffer) or check_length < 0) - { - return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, - memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); - } - write_length+= (size_t)check_length; - WATCHPOINT_ASSERT(write_length < MEMCACHED_DEFAULT_COMMAND_SIZE); - } + struct libmemcached_io_vector_st vector[]= + { + { storage_op_string(verb), strlen(storage_op_string(verb))}, + { memcached_array_string(ptr->_namespace), memcached_array_size(ptr->_namespace) }, + { key, key_length }, + { flags_buffer, flags_buffer_length }, + { expiration_buffer, expiration_buffer_length }, + { value_buffer, value_buffer_length }, + { cas_buffer, cas_buffer_length }, + { " noreply", reply ? 0 : memcached_literal_param_size(" noreply") }, + { memcached_literal_param("\r\n") }, + { value, value_length }, + { memcached_literal_param("\r\n") } + }; - if (ptr->flags.use_udp) + if (memcached_is_udp(instance->root)) { + size_t write_length= io_vector_total_size(vector, 11); + size_t cmd_size= write_length + value_length +2; if (cmd_size > MAX_UDP_DATAGRAM_LENGTH - UDP_DATAGRAM_HEADER_LENGTH) { @@ -317,25 +318,8 @@ static memcached_return_t memcached_send_ascii(memcached_st *ptr, } } - if (write_length >= MEMCACHED_DEFAULT_COMMAND_SIZE) - { - return memcached_set_error(*ptr, MEMCACHED_WRITE_FAILURE, MEMCACHED_AT); - } - - struct libmemcached_io_vector_st vector[]= - { - { buffer, write_length }, - { value, value_length }, - { memcached_literal_param("\r\n") } - }; - - if (memcached_is_udp(instance->root) and (write_length +value_length +memcached_literal_param_size("\r\n") +UDP_DATAGRAM_HEADER_LENGTH > MAX_UDP_DATAGRAM_LENGTH)) - { - return memcached_set_error(*instance, MEMCACHED_WRITE_FAILURE, MEMCACHED_AT, memcached_literal_param("UDP packet is too large")); - } - /* Send command header */ - memcached_return_t rc= memcached_vdo(instance, vector, 3, flush); + memcached_return_t rc= memcached_vdo(instance, vector, 11, flush); if (rc == MEMCACHED_SUCCESS) { if (ptr->flags.no_reply and flush) @@ -348,6 +332,7 @@ static memcached_return_t memcached_send_ascii(memcached_st *ptr, } else { + char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; rc= memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, NULL); if (rc == MEMCACHED_STORED) @@ -374,9 +359,9 @@ static inline memcached_return_t memcached_send(memcached_st *ptr, const char *group_key, size_t group_key_length, const char *key, size_t key_length, const char *value, size_t value_length, - time_t expiration, - uint32_t flags, - uint64_t cas, + const time_t expiration, + const uint32_t flags, + const uint64_t cas, memcached_storage_action_t verb) { memcached_return_t rc; diff --git a/tests/libmemcached-1.0/mem_functions.cc b/tests/libmemcached-1.0/mem_functions.cc index 2da5d046..fc8a71ba 100644 --- a/tests/libmemcached-1.0/mem_functions.cc +++ b/tests/libmemcached-1.0/mem_functions.cc @@ -522,7 +522,7 @@ static test_return_t set_test(memcached_st *memc) test_literal_param("foo"), test_literal_param("when we sanitize"), time_t(0), (uint32_t)0); - test_true_got(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED, memcached_strerror(NULL, rc)); + test_true_got(rc == MEMCACHED_SUCCESS or rc == MEMCACHED_BUFFERED, memcached_strerror(NULL, rc)); return TEST_SUCCESS; } @@ -653,7 +653,6 @@ static test_return_t cas2_test(memcached_st *memc) static test_return_t cas_test(memcached_st *memc) { - memcached_return_t rc; const char *key= "fun"; size_t key_length= strlen(key); const char *value= "we the people"; @@ -665,17 +664,15 @@ static test_return_t cas_test(memcached_st *memc) memcached_result_st results_obj; memcached_result_st *results; - unsigned int set= 1; - rc= memcached_flush(memc, 0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, memcached_flush(memc, 0)); - memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_SUPPORT_CAS, set); + test_skip(true, memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_SUPPORT_CAS, true)); - rc= memcached_set(memc, key, strlen(key), - value, strlen(value), - (time_t)0, (uint32_t)0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, + memcached_set(memc, key, strlen(key), + value, strlen(value), + (time_t)0, (uint32_t)0)); test_compare(MEMCACHED_SUCCESS, memcached_mget(memc, keys, keylengths, 1)); @@ -683,6 +680,7 @@ static test_return_t cas_test(memcached_st *memc) results= memcached_result_create(memc, &results_obj); test_true(results); + memcached_return_t rc; results= memcached_fetch_result(memc, &results_obj, &rc); test_true(results); test_compare(MEMCACHED_SUCCESS, rc); @@ -715,32 +713,31 @@ static test_return_t cas_test(memcached_st *memc) static test_return_t prepend_test(memcached_st *memc) { - memcached_return_t rc; const char *key= "fig"; const char *value= "people"; - char *out_value= NULL; - size_t value_length; - uint32_t flags; - rc= memcached_flush(memc, 0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, + memcached_flush(memc, 0)); - rc= memcached_set(memc, key, strlen(key), - value, strlen(value), - (time_t)0, (uint32_t)0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, + memcached_set(memc, key, strlen(key), + value, strlen(value), + time_t(0), uint32_t(0))); - rc= memcached_prepend(memc, key, strlen(key), - "the ", strlen("the "), - (time_t)0, (uint32_t)0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, + memcached_prepend(memc, key, strlen(key), + "the ", strlen("the "), + time_t(0), uint32_t(0))); - rc= memcached_prepend(memc, key, strlen(key), - "we ", strlen("we "), - (time_t)0, (uint32_t)0); - test_compare(MEMCACHED_SUCCESS, rc); + test_compare(MEMCACHED_SUCCESS, + memcached_prepend(memc, key, strlen(key), + "we ", strlen("we "), + time_t(0), uint32_t(0))); - out_value= memcached_get(memc, key, strlen(key), + size_t value_length; + uint32_t flags; + memcached_return_t rc; + char *out_value= memcached_get(memc, key, strlen(key), &value_length, &flags, &rc); test_memcmp(out_value, "we the people", strlen("we the people")); test_compare(strlen("we the people"), value_length); @@ -766,7 +763,7 @@ static test_return_t add_test(memcached_st *memc) rc= memcached_set(memc, key, strlen(key), value, strlen(value), (time_t)0, (uint32_t)0); - test_true(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED); + test_true(rc == MEMCACHED_SUCCESS or rc == MEMCACHED_BUFFERED); memcached_quit(memc); rc= memcached_add(memc, key, strlen(key), value, strlen(value), @@ -775,11 +772,11 @@ static test_return_t add_test(memcached_st *memc) /* Too many broken OS'es have broken loopback in async, so we can't be sure of the result */ if (setting_value) { - test_true(rc == MEMCACHED_NOTSTORED || rc == MEMCACHED_STORED); + test_true(rc == MEMCACHED_NOTSTORED or rc == MEMCACHED_STORED); } else { - test_true(rc == MEMCACHED_NOTSTORED || rc == MEMCACHED_DATA_EXISTS); + test_true(rc == MEMCACHED_NOTSTORED or rc == MEMCACHED_DATA_EXISTS); } return TEST_SUCCESS; @@ -2494,37 +2491,36 @@ static test_return_t user_supplied_bug8(memcached_st *) /* Test flag store/retrieve */ static test_return_t user_supplied_bug7(memcached_st *memc) { - const char *keys= "036790384900"; - size_t key_length= strlen(keys); - char return_key[MEMCACHED_MAX_KEY]; - size_t return_key_length; - char *value; - size_t value_length; - uint32_t flags; char *insert_data= new (std::nothrow) char[VALUE_SIZE_BUG5]; + test_true(insert_data); - for (unsigned int x= 0; x < VALUE_SIZE_BUG5; x++) + for (size_t x= 0; x < VALUE_SIZE_BUG5; x++) { insert_data[x]= (signed char)rand(); } memcached_flush(memc, 0); - flags= 245; - test_compare(MEMCACHED_SUCCESS, memcached_set(memc, keys, key_length, - insert_data, VALUE_SIZE_BUG5, - (time_t)0, flags)); + const char *keys= "036790384900"; + size_t key_length= strlen(keys); + test_compare_hint(MEMCACHED_SUCCESS, memcached_set(memc, keys, key_length, + insert_data, VALUE_SIZE_BUG5, + time_t(0), 245U), + memcached_last_error_message(memc)); memcached_return_t rc; - flags= 0; - value= memcached_get(memc, keys, key_length, - &value_length, &flags, &rc); + size_t value_length; + uint32_t flags= 0; + char *value= memcached_get(memc, keys, key_length, + &value_length, &flags, &rc); test_compare(245U, flags); test_true(value); free(value); test_compare(MEMCACHED_SUCCESS, memcached_mget(memc, &keys, &key_length, 1)); + char return_key[MEMCACHED_MAX_KEY]; + size_t return_key_length; flags= 0; value= memcached_fetch(memc, return_key, &return_key_length, &value_length, &flags, &rc); @@ -2806,19 +2802,20 @@ static test_return_t user_supplied_bug15(memcached_st *memc) /* Check the return sizes on FLAGS to make sure it stores 32bit unsigned values correctly */ static test_return_t user_supplied_bug16(memcached_st *memc) { - memcached_return_t rc= memcached_set(memc, test_literal_param("mykey"), - NULL, 0, - (time_t)0, UINT32_MAX); + test_compare_hint(MEMCACHED_SUCCESS, memcached_set(memc, test_literal_param("mykey"), + NULL, 0, + (time_t)0, UINT32_MAX), + memcached_last_error_message(memc)); - test_compare(MEMCACHED_SUCCESS, rc); size_t length; uint32_t flags; + memcached_return_t rc; char *value= memcached_get(memc, test_literal_param("mykey"), &length, &flags, &rc); test_compare(MEMCACHED_SUCCESS, rc); - test_true(value == NULL); + test_null(value); test_zero(length); test_compare(flags, UINT32_MAX); -- 2.30.2