From 4b3ecdf765b5d15e3ae65e6d977d56d2272bddd0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 4 Dec 2015 09:33:05 -0800 Subject: [PATCH 01/12] make grpc compile on win64bit --- src/core/iomgr/tcp_windows.c | 10 ++++++---- src/core/support/string.c | 21 ++++++++++++++++++++ src/core/support/string.h | 10 ++++++++++ src/core/transport/chttp2/timeout_encoding.c | 7 ++++--- 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/core/iomgr/tcp_windows.c b/src/core/iomgr/tcp_windows.c index 5ff78231bd7..db8e7f56fe9 100644 --- a/src/core/iomgr/tcp_windows.c +++ b/src/core/iomgr/tcp_windows.c @@ -197,7 +197,7 @@ static void win_read(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, tcp->read_slice = gpr_slice_malloc(8192); - buffer.len = GPR_SLICE_LENGTH(tcp->read_slice); + buffer.len = (ULONG)GPR_SLICE_LENGTH(tcp->read_slice); // we know slice size fits in 32bit. buffer.buf = (char *)GPR_SLICE_START_PTR(tcp->read_slice); TCP_REF(tcp, "read"); @@ -282,18 +282,20 @@ static void win_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, tcp->write_cb = cb; tcp->write_slices = slices; + GPR_ASSERT((tcp->write_slices->count >> 32) == 0); if (tcp->write_slices->count > GPR_ARRAY_SIZE(local_buffers)) { buffers = (WSABUF *)gpr_malloc(sizeof(WSABUF) * tcp->write_slices->count); allocated = buffers; } for (i = 0; i < tcp->write_slices->count; i++) { - buffers[i].len = GPR_SLICE_LENGTH(tcp->write_slices->slices[i]); + GPR_ASSERT((GPR_SLICE_LENGTH(tcp->write_slices->slices[i]) >> 32) == 0); + buffers[i].len = (ULONG) GPR_SLICE_LENGTH(tcp->write_slices->slices[i]); buffers[i].buf = (char *)GPR_SLICE_START_PTR(tcp->write_slices->slices[i]); } /* First, let's try a synchronous, non-blocking write. */ - status = WSASend(socket->socket, buffers, tcp->write_slices->count, + status = WSASend(socket->socket, buffers, (DWORD)tcp->write_slices->count, &bytes_sent, 0, NULL, NULL); info->wsa_error = status == 0 ? 0 : WSAGetLastError(); @@ -322,7 +324,7 @@ static void win_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, /* If we got a WSAEWOULDBLOCK earlier, then we need to re-do the same operation, this time asynchronously. */ memset(&socket->write_info.overlapped, 0, sizeof(OVERLAPPED)); - status = WSASend(socket->socket, buffers, tcp->write_slices->count, + status = WSASend(socket->socket, buffers, (DWORD)tcp->write_slices->count, &bytes_sent, 0, &socket->write_info.overlapped, NULL); if (allocated) gpr_free(allocated); diff --git a/src/core/support/string.c b/src/core/support/string.c index e0ffeb8a4af..fad0b4008bd 100644 --- a/src/core/support/string.c +++ b/src/core/support/string.c @@ -173,6 +173,27 @@ int gpr_ltoa(long value, char *string) { return i; } +int gpr_int64toa(gpr_int64 value, char *string) { + int i = 0; + int neg = value < 0; + + if (value == 0) { + string[0] = '0'; + string[1] = 0; + return 1; + } + + if (neg) value = -value; + while (value) { + string[i++] = (char)('0' + value % 10); + value /= 10; + } + if (neg) string[i++] = '-'; + gpr_reverse_bytes(string, i); + string[i] = 0; + return i; +} + char *gpr_strjoin(const char **strs, size_t nstrs, size_t *final_length) { return gpr_strjoin_sep(strs, nstrs, "", final_length); } diff --git a/src/core/support/string.h b/src/core/support/string.h index a28e00fd3ec..9b604ac5bf5 100644 --- a/src/core/support/string.h +++ b/src/core/support/string.h @@ -70,6 +70,16 @@ int gpr_parse_bytes_to_uint32(const char *data, size_t length, output must be at least GPR_LTOA_MIN_BUFSIZE bytes long. */ int gpr_ltoa(long value, char *output); +/* Minimum buffer size for calling int64toa */ +#define GPR_INT64TOA_MIN_BUFSIZE (3 * sizeof(gpr_int64)) + +/* Convert an int64 to a string in base 10; returns the length of the +output string (or 0 on failure). +output must be at least GPR_INT64TOA_MIN_BUFSIZE bytes long. +NOTE: This function ensures sufficient bit width even on Win x64, +where long is 32bit is size.*/ +int gpr_int64toa(gpr_int64 value, char *output); + /* Reverse a run of bytes */ void gpr_reverse_bytes(char *str, int len); diff --git a/src/core/transport/chttp2/timeout_encoding.c b/src/core/transport/chttp2/timeout_encoding.c index 8a9b290ecb9..cf81c18a20b 100644 --- a/src/core/transport/chttp2/timeout_encoding.c +++ b/src/core/transport/chttp2/timeout_encoding.c @@ -36,6 +36,7 @@ #include #include +#include #include "src/core/support/string.h" static int round_up(int x, int divisor) { @@ -57,13 +58,13 @@ static int round_up_to_three_sig_figs(int x) { /* encode our minimum viable timeout value */ static void enc_tiny(char *buffer) { memcpy(buffer, "1n", 3); } -static void enc_ext(char *buffer, long value, char ext) { - int n = gpr_ltoa(value, buffer); +static void enc_ext(char *buffer, gpr_int64 value, char ext) { + int n = gpr_int64toa(value, buffer); buffer[n] = ext; buffer[n + 1] = 0; } -static void enc_seconds(char *buffer, long sec) { +static void enc_seconds(char *buffer, gpr_int64 sec) { if (sec % 3600 == 0) { enc_ext(buffer, sec / 3600, 'H'); } else if (sec % 60 == 0) { From da9b8ae031bd1b94ae6bd9d91c10dae1e6cc98df Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 4 Dec 2015 10:10:38 -0800 Subject: [PATCH 02/12] fix 32 bit assertions --- src/core/iomgr/tcp_windows.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/iomgr/tcp_windows.c b/src/core/iomgr/tcp_windows.c index db8e7f56fe9..b768bc89b62 100644 --- a/src/core/iomgr/tcp_windows.c +++ b/src/core/iomgr/tcp_windows.c @@ -281,15 +281,14 @@ static void win_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, tcp->write_cb = cb; tcp->write_slices = slices; - - GPR_ASSERT((tcp->write_slices->count >> 32) == 0); + GPR_ASSERT(tcp->write_slices->count <= UINT32_MAX); if (tcp->write_slices->count > GPR_ARRAY_SIZE(local_buffers)) { buffers = (WSABUF *)gpr_malloc(sizeof(WSABUF) * tcp->write_slices->count); allocated = buffers; } for (i = 0; i < tcp->write_slices->count; i++) { - GPR_ASSERT((GPR_SLICE_LENGTH(tcp->write_slices->slices[i]) >> 32) == 0); + GPR_ASSERT(GPR_SLICE_LENGTH(tcp->write_slices->slices[i]) <= UINT32_MAX); buffers[i].len = (ULONG) GPR_SLICE_LENGTH(tcp->write_slices->slices[i]); buffers[i].buf = (char *)GPR_SLICE_START_PTR(tcp->write_slices->slices[i]); } From 52b0f6c371ce184bf9dfe4ac635f95b48b32ea53 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 4 Dec 2015 10:36:27 -0800 Subject: [PATCH 03/12] use int64toa for tv_sec --- src/core/security/json_token.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/security/json_token.c b/src/core/security/json_token.c index 021912f3337..92775d885d2 100644 --- a/src/core/security/json_token.c +++ b/src/core/security/json_token.c @@ -215,8 +215,8 @@ static char *encoded_jwt_claim(const grpc_auth_json_key *json_key, gpr_log(GPR_INFO, "Cropping token lifetime to maximum allowed value."); expiration = gpr_time_add(now, grpc_max_auth_token_lifetime); } - gpr_ltoa(now.tv_sec, now_str); - gpr_ltoa(expiration.tv_sec, expiration_str); + gpr_int64toa(now.tv_sec, now_str); + gpr_int64toa(expiration.tv_sec, expiration_str); child = create_child(NULL, json, "iss", json_key->client_email, GRPC_JSON_STRING); From 5b155e56746d50e0f6db1492c448d783db9dcdac Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 4 Dec 2015 16:57:06 -0800 Subject: [PATCH 04/12] more win x64 fixes --- include/grpc++/impl/rpc_service_method.h | 6 +++++- src/cpp/proto/proto_utils.cc | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/grpc++/impl/rpc_service_method.h b/include/grpc++/impl/rpc_service_method.h index fcb0b7ccce6..9d30d9627cc 100644 --- a/include/grpc++/impl/rpc_service_method.h +++ b/include/grpc++/impl/rpc_service_method.h @@ -251,7 +251,11 @@ class RpcService { void AddMethod(RpcServiceMethod* method) { methods_.emplace_back(method); } RpcServiceMethod* GetMethod(int i) { return methods_[i].get(); } - int GetMethodCount() const { return methods_.size(); } + int GetMethodCount() const { + // On win x64, int is only 32bit + GPR_ASSERT(methods_.size() <= INT_MAX); + return (int)methods_.size(); + } private: std::vector> methods_; diff --git a/src/cpp/proto/proto_utils.cc b/src/cpp/proto/proto_utils.cc index b1330fde7f6..0a05f292221 100644 --- a/src/cpp/proto/proto_utils.cc +++ b/src/cpp/proto/proto_utils.cc @@ -70,7 +70,9 @@ class GrpcBufferWriter GRPC_FINAL slice_ = gpr_slice_malloc(block_size_); } *data = GPR_SLICE_START_PTR(slice_); - byte_count_ += * size = GPR_SLICE_LENGTH(slice_); + // On win x64, int is only 32bit + GPR_ASSERT(GPR_SLICE_LENGTH(slice_) <= INT_MAX); + byte_count_ += * size = (int)GPR_SLICE_LENGTH(slice_); gpr_slice_buffer_add(slice_buffer_, slice_); return true; } @@ -124,7 +126,9 @@ class GrpcBufferReader GRPC_FINAL } gpr_slice_unref(slice_); *data = GPR_SLICE_START_PTR(slice_); - byte_count_ += * size = GPR_SLICE_LENGTH(slice_); + // On win x64, int is only 32bit + GPR_ASSERT(GPR_SLICE_LENGTH(slice_) <= INT_MAX); + byte_count_ += * size = (int)GPR_SLICE_LENGTH(slice_); return true; } From 2dd156ef642fa2968437a736252532303ae0f022 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 4 Dec 2015 18:26:17 -0800 Subject: [PATCH 05/12] add --arch, --compiler and --build_only cmdline args --- tools/run_tests/jobset.py | 3 + tools/run_tests/run_tests.py | 75 ++++++++++++++++++++-- vsprojects/build_vs2010.bat | 10 +++ vsprojects/{build.bat => build_vs2013.bat} | 0 vsprojects/build_vs2015.bat | 10 +++ 5 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 vsprojects/build_vs2010.bat rename vsprojects/{build.bat => build_vs2013.bat} (100%) create mode 100644 vsprojects/build_vs2015.bat diff --git a/tools/run_tests/jobset.py b/tools/run_tests/jobset.py index 5a26bff709c..01739be27c9 100755 --- a/tools/run_tests/jobset.py +++ b/tools/run_tests/jobset.py @@ -178,6 +178,9 @@ class JobSpec(object): def __cmp__(self, other): return self.identity() == other.identity() + + def __repr__(self): + return 'JobSpec(shortname=%s, cmdline=%s)' % (self.shortname, self.cmdline) class JobResult(object): diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 4c85f202f47..e3df9124803 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -508,6 +508,43 @@ _WINDOWS_CONFIG = { } +def _windows_arch_option(arch): + """Returns msbuild cmdline option for selected architecture.""" + if arch == 'default' or arch == 'windows_x86': + return '/p:Platform=Win32' + elif arch == 'windows_x64': + return '/p:Platform=x64' + else: + print 'Architecture %s not supported on current platform.' % arch + sys.exit(1) + + +def _windows_build_bat(compiler): + """Returns name of build.bat for selected compiler.""" + if compiler == 'default' or compiler == 'vs2013': + return 'vsprojects\\build_vs2013.bat' + elif compiler == 'vs2015': + return 'vsprojects\\build_vs2015.bat' + elif compiler == 'vs2010': + return 'vsprojects\\build_vs2010.bat' + else: + print 'Compiler %s not supported.' % compiler + sys.exit(1) + + +def _windows_toolset_option(compiler): + """Returns msbuild PlatformToolset for selected compiler.""" + if compiler == 'default' or compiler == 'vs2013': + return '/p:PlatformToolset=v120' + elif compiler == 'vs2015': + return '/p:PlatformToolset=v140' + elif compiler == 'vs2010': + return '/p:PlatformToolset=v100' + else: + print 'Compiler %s not supported.' % compiler + sys.exit(1) + + def runs_per_test_type(arg_str): """Auxilary function to parse the "runs_per_test" flag. @@ -572,6 +609,19 @@ argp.add_argument('--allow_flakes', action='store_const', const=True, help='Allow flaky tests to show as passing (re-runs failed tests up to five times)') +argp.add_argument('--arch', + choices=['default', 'windows_x86', 'windows_x64'], + default='default', + help='Selects architecture to target. For some platforms "default" is the only supported choice.') +argp.add_argument('--compiler', + choices=['default', 'vs2010', 'vs2013', 'vs2015'], + default='default', + help='Selects compiler to use. For some platforms "default" is the only supported choice.') +argp.add_argument('--build_only', + default=False, + action='store_const', + const=True, + help='Perform all the build steps but dont run any tests.') argp.add_argument('-a', '--antagonists', default=0, type=int) argp.add_argument('-x', '--xml_report', default=None, type=str, help='Generates a JUnit-compatible XML report') @@ -633,6 +683,14 @@ if len(build_configs) > 1: print language, 'does not support multiple build configurations' sys.exit(1) +if platform_string() != 'windows': + if args.arch != 'default': + print 'Architecture %s not supported on current platform.' % args.arch + sys.exit(1) + if args.compiler != 'default': + print 'Compiler %s not supported on current platform.' % args.compiler + sys.exit(1) + if platform_string() == 'windows': def make_jobspec(cfg, targets, makefile='Makefile'): extra_args = [] @@ -643,9 +701,11 @@ if platform_string() == 'windows': # disable PDB generation: it's broken, and we don't need it during CI extra_args.extend(['/p:Jenkins=true']) return [ - jobset.JobSpec(['vsprojects\\build.bat', + jobset.JobSpec([_windows_build_bat(args.compiler), 'vsprojects\\%s.sln' % target, - '/p:Configuration=%s' % _WINDOWS_CONFIG[cfg]] + + '/p:Configuration=%s' % _WINDOWS_CONFIG[cfg], + _windows_toolset_option(args.compiler), + _windows_arch_option(args.arch)] + extra_args, shell=True, timeout_seconds=90*60) for target in targets] @@ -840,7 +900,7 @@ def _calculate_num_runs_failures(list_of_results): def _build_and_run( - check_cancelled, newline_on_success, cache, xml_report=None): + check_cancelled, newline_on_success, cache, xml_report=None, build_only=False): """Do one pass of building & running tests.""" # build latest sequentially num_failures, _ = jobset.run( @@ -848,6 +908,9 @@ def _build_and_run( newline_on_success=newline_on_success, travis=args.travis) if num_failures: return 1 + + if build_only: + return 0 # start antagonists antagonists = [subprocess.Popen(['tools/run_tests/antagonist.py']) @@ -925,7 +988,8 @@ if forever: previous_success = success success = _build_and_run(check_cancelled=have_files_changed, newline_on_success=False, - cache=test_cache) == 0 + cache=test_cache, + build_only=args.build_only) == 0 if not previous_success and success: jobset.message('SUCCESS', 'All tests are now passing properly', @@ -937,7 +1001,8 @@ else: result = _build_and_run(check_cancelled=lambda: False, newline_on_success=args.newline_on_success, cache=test_cache, - xml_report=args.xml_report) + xml_report=args.xml_report, + build_only=args.build_only) if result == 0: jobset.message('SUCCESS', 'All tests passed', do_newline=True) else: diff --git a/vsprojects/build_vs2010.bat b/vsprojects/build_vs2010.bat new file mode 100644 index 00000000000..64b0ed5d3f5 --- /dev/null +++ b/vsprojects/build_vs2010.bat @@ -0,0 +1,10 @@ +@rem Convenience wrapper that runs specified gRPC target using msbuild +@rem Usage: build.bat TARGET_NAME + +setlocal +@rem Set VS variables (uses Visual Studio 2010) +@call "%VS100COMNTOOLS%\..\..\vc\vcvarsall.bat" x86 + +msbuild %* +exit /b %ERRORLEVEL% +endlocal diff --git a/vsprojects/build.bat b/vsprojects/build_vs2013.bat similarity index 100% rename from vsprojects/build.bat rename to vsprojects/build_vs2013.bat diff --git a/vsprojects/build_vs2015.bat b/vsprojects/build_vs2015.bat new file mode 100644 index 00000000000..50485a30f30 --- /dev/null +++ b/vsprojects/build_vs2015.bat @@ -0,0 +1,10 @@ +@rem Convenience wrapper that runs specified gRPC target using msbuild +@rem Usage: build.bat TARGET_NAME + +setlocal +@rem Set VS variables (uses Visual Studio 2015) +@call "%VS140COMNTOOLS%\..\..\vc\vcvarsall.bat" x86 + +msbuild %* +exit /b %ERRORLEVEL% +endlocal From 5dc93c363b7422a0baae67f2db32713c264f405b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 7 Dec 2015 08:11:27 -0800 Subject: [PATCH 06/12] include climits --- include/grpc++/impl/rpc_service_method.h | 1 + src/cpp/proto/proto_utils.cc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/include/grpc++/impl/rpc_service_method.h b/include/grpc++/impl/rpc_service_method.h index 9d30d9627cc..b203c8f53ae 100644 --- a/include/grpc++/impl/rpc_service_method.h +++ b/include/grpc++/impl/rpc_service_method.h @@ -34,6 +34,7 @@ #ifndef GRPCXX_IMPL_RPC_SERVICE_METHOD_H #define GRPCXX_IMPL_RPC_SERVICE_METHOD_H +#include #include #include #include diff --git a/src/cpp/proto/proto_utils.cc b/src/cpp/proto/proto_utils.cc index 0a05f292221..898a1d4f581 100644 --- a/src/cpp/proto/proto_utils.cc +++ b/src/cpp/proto/proto_utils.cc @@ -33,6 +33,8 @@ #include +#include + #include #include #include From f67f071f7e1fadd144a867eb7fbe6551cbfcc3f5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 7 Dec 2015 18:09:49 -0800 Subject: [PATCH 07/12] address comments --- src/core/iomgr/tcp_windows.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/iomgr/tcp_windows.c b/src/core/iomgr/tcp_windows.c index b768bc89b62..6915cb2f493 100644 --- a/src/core/iomgr/tcp_windows.c +++ b/src/core/iomgr/tcp_windows.c @@ -273,6 +273,7 @@ static void win_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, WSABUF local_buffers[16]; WSABUF *allocated = NULL; WSABUF *buffers = local_buffers; + size_t len; if (tcp->shutting_down) { grpc_exec_ctx_enqueue(exec_ctx, cb, 0); @@ -281,15 +282,16 @@ static void win_write(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep, tcp->write_cb = cb; tcp->write_slices = slices; - GPR_ASSERT(tcp->write_slices->count <= UINT32_MAX); + GPR_ASSERT(tcp->write_slices->count <= UINT_MAX); if (tcp->write_slices->count > GPR_ARRAY_SIZE(local_buffers)) { buffers = (WSABUF *)gpr_malloc(sizeof(WSABUF) * tcp->write_slices->count); allocated = buffers; } for (i = 0; i < tcp->write_slices->count; i++) { - GPR_ASSERT(GPR_SLICE_LENGTH(tcp->write_slices->slices[i]) <= UINT32_MAX); - buffers[i].len = (ULONG) GPR_SLICE_LENGTH(tcp->write_slices->slices[i]); + len = GPR_SLICE_LENGTH(tcp->write_slices->slices[i]); + GPR_ASSERT(len <= ULONG_MAX); + buffers[i].len = (ULONG) len; buffers[i].buf = (char *)GPR_SLICE_START_PTR(tcp->write_slices->slices[i]); } From 338ef4aa6cf39be4761b39b25ce5dec829af535b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 8 Dec 2015 17:48:17 -0800 Subject: [PATCH 08/12] fix bug MIN_VALUE bug in ltoa and int64toa and add tests --- src/core/support/string.c | 26 +++++++++++------ test/core/support/string_test.c | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/src/core/support/string.c b/src/core/support/string.c index fad0b4008bd..bebeeb95138 100644 --- a/src/core/support/string.c +++ b/src/core/support/string.c @@ -153,6 +153,7 @@ void gpr_reverse_bytes(char *str, int len) { } int gpr_ltoa(long value, char *string) { + unsigned long uval; int i = 0; int neg = value < 0; @@ -162,10 +163,14 @@ int gpr_ltoa(long value, char *string) { return 1; } - if (neg) value = -value; - while (value) { - string[i++] = (char)('0' + value % 10); - value /= 10; + if (neg) { + uval = -value; + } else { + uval = value; + } + while (uval) { + string[i++] = (char)('0' + uval % 10); + uval /= 10; } if (neg) string[i++] = '-'; gpr_reverse_bytes(string, i); @@ -174,6 +179,7 @@ int gpr_ltoa(long value, char *string) { } int gpr_int64toa(gpr_int64 value, char *string) { + gpr_uint64 uval; int i = 0; int neg = value < 0; @@ -183,10 +189,14 @@ int gpr_int64toa(gpr_int64 value, char *string) { return 1; } - if (neg) value = -value; - while (value) { - string[i++] = (char)('0' + value % 10); - value /= 10; + if (neg) { + uval = -value; + } else { + uval = value; + } + while (uval) { + string[i++] = (char)('0' + uval % 10); + uval /= 10; } if (neg) string[i++] = '-'; gpr_reverse_bytes(string, i); diff --git a/test/core/support/string_test.c b/test/core/support/string_test.c index f62cbe34358..b4024fb7995 100644 --- a/test/core/support/string_test.c +++ b/test/core/support/string_test.c @@ -286,6 +286,53 @@ static void test_strsplit(void) { gpr_free(parts); } +test_ltoa() { + char *str; + char buf[GPR_LTOA_MIN_BUFSIZE]; + + LOG_TEST_NAME("test_ltoa"); + + /* zero */ + GPR_ASSERT(1 == gpr_ltoa(0, buf)); + GPR_ASSERT(0 == strcmp("0", buf)); + + /* positive number */ + GPR_ASSERT(3 == gpr_ltoa(123, buf)); + GPR_ASSERT(0 == strcmp("123", buf)); + + /* negative number */ + GPR_ASSERT(6 == gpr_ltoa(-12345, buf)); + GPR_ASSERT(0 == strcmp("-12345", buf)); + + /* large negative - we don't know the size of long in advance */ + GPR_ASSERT(gpr_asprintf(&str, "%lld", (long long)LONG_MIN)); + GPR_ASSERT(strlen(str) == gpr_ltoa(LONG_MIN, buf)); + GPR_ASSERT(0 == strcmp(str, buf)); + gpr_free(str); +} + +test_int64toa() { + char buf[GPR_INT64TOA_MIN_BUFSIZE]; + + LOG_TEST_NAME("test_int64toa"); + + /* zero */ + GPR_ASSERT(1 == gpr_int64toa(0, buf)); + GPR_ASSERT(0 == strcmp("0", buf)); + + /* positive */ + GPR_ASSERT(3 == gpr_int64toa(123, buf)); + GPR_ASSERT(0 == strcmp("123", buf)); + + /* large positive */ + GPR_ASSERT(19 == gpr_int64toa(9223372036854775807LL, buf)); + GPR_ASSERT(0 == strcmp("9223372036854775807", buf)); + + /* large negative */ + GPR_ASSERT(20 == gpr_int64toa(-9223372036854775808LL, buf)); + GPR_ASSERT(0 == strcmp("-9223372036854775808", buf)); +} + int main(int argc, char **argv) { grpc_test_init(argc, argv); test_strdup(); @@ -296,5 +343,7 @@ int main(int argc, char **argv) { test_strjoin(); test_strjoin_sep(); test_strsplit(); + test_ltoa(); + test_int64toa(); return 0; } From 26e190f3ee7c259165ce12106172f3bd25b1e323 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 8 Dec 2015 22:44:21 -0800 Subject: [PATCH 09/12] make test methods static void --- test/core/support/string_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/support/string_test.c b/test/core/support/string_test.c index b4024fb7995..0ae930fa615 100644 --- a/test/core/support/string_test.c +++ b/test/core/support/string_test.c @@ -286,7 +286,7 @@ static void test_strsplit(void) { gpr_free(parts); } -test_ltoa() { +static void test_ltoa() { char *str; char buf[GPR_LTOA_MIN_BUFSIZE]; @@ -311,7 +311,7 @@ test_ltoa() { gpr_free(str); } -test_int64toa() { +static void test_int64toa() { char buf[GPR_INT64TOA_MIN_BUFSIZE]; LOG_TEST_NAME("test_int64toa"); From 4121f7a60f96f42839321988c35e4d14a34e5956 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 9 Dec 2015 11:02:54 -0800 Subject: [PATCH 10/12] explicitly cast signed to unsigned where safe --- src/core/support/string.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/support/string.c b/src/core/support/string.c index bebeeb95138..ed000e33724 100644 --- a/src/core/support/string.c +++ b/src/core/support/string.c @@ -164,9 +164,9 @@ int gpr_ltoa(long value, char *string) { } if (neg) { - uval = -value; + uval = (unsigned long)-value; } else { - uval = value; + uval = (unsigned long)value; } while (uval) { string[i++] = (char)('0' + uval % 10); @@ -190,9 +190,9 @@ int gpr_int64toa(gpr_int64 value, char *string) { } if (neg) { - uval = -value; + uval = (gpr_uint64)-value; } else { - uval = value; + uval = (gpr_uint64)value; } while (uval) { string[i++] = (char)('0' + uval % 10); From cc0dd226c6463ea9a8d0f39d256746a7f918a597 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 9 Dec 2015 11:49:33 -0800 Subject: [PATCH 11/12] reimplement ltoa and int64toa without negation --- src/core/support/string.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/core/support/string.c b/src/core/support/string.c index ed000e33724..46a7ca3d462 100644 --- a/src/core/support/string.c +++ b/src/core/support/string.c @@ -153,9 +153,8 @@ void gpr_reverse_bytes(char *str, int len) { } int gpr_ltoa(long value, char *string) { - unsigned long uval; + long sign; int i = 0; - int neg = value < 0; if (value == 0) { string[0] = '0'; @@ -163,25 +162,20 @@ int gpr_ltoa(long value, char *string) { return 1; } - if (neg) { - uval = (unsigned long)-value; - } else { - uval = (unsigned long)value; + sign = value < 0 ? -1 : 1; + while (value) { + string[i++] = (char)('0' + sign * (value % 10)); + value /= 10; } - while (uval) { - string[i++] = (char)('0' + uval % 10); - uval /= 10; - } - if (neg) string[i++] = '-'; + if (sign < 0) string[i++] = '-'; gpr_reverse_bytes(string, i); string[i] = 0; return i; } int gpr_int64toa(gpr_int64 value, char *string) { - gpr_uint64 uval; + gpr_int64 sign; int i = 0; - int neg = value < 0; if (value == 0) { string[0] = '0'; @@ -189,16 +183,12 @@ int gpr_int64toa(gpr_int64 value, char *string) { return 1; } - if (neg) { - uval = (gpr_uint64)-value; - } else { - uval = (gpr_uint64)value; - } - while (uval) { - string[i++] = (char)('0' + uval % 10); - uval /= 10; + sign = value < 0 ? -1 : 1; + while (value) { + string[i++] = (char)('0' + sign * (value % 10)); + value /= 10; } - if (neg) string[i++] = '-'; + if (sign < 0) string[i++] = '-'; gpr_reverse_bytes(string, i); string[i] = 0; return i; From 06ae5cd48c1e235b0980cfca631c94572c7777d8 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 9 Dec 2015 19:24:04 -0800 Subject: [PATCH 12/12] get rid of warnings on linux --- test/core/support/string_test.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/core/support/string_test.c b/test/core/support/string_test.c index 0ae930fa615..c97d3176c5d 100644 --- a/test/core/support/string_test.c +++ b/test/core/support/string_test.c @@ -33,6 +33,7 @@ #include "src/core/support/string.h" +#include #include #include #include @@ -306,7 +307,7 @@ static void test_ltoa() { /* large negative - we don't know the size of long in advance */ GPR_ASSERT(gpr_asprintf(&str, "%lld", (long long)LONG_MIN)); - GPR_ASSERT(strlen(str) == gpr_ltoa(LONG_MIN, buf)); + GPR_ASSERT(strlen(str) == (size_t)gpr_ltoa(LONG_MIN, buf)); GPR_ASSERT(0 == strcmp(str, buf)); gpr_free(str); } @@ -329,7 +330,7 @@ static void test_int64toa() { GPR_ASSERT(0 == strcmp("9223372036854775807", buf)); /* large negative */ - GPR_ASSERT(20 == gpr_int64toa(-9223372036854775808LL, buf)); + GPR_ASSERT(20 == gpr_int64toa(-9223372036854775807LL - 1, buf)); GPR_ASSERT(0 == strcmp("-9223372036854775808", buf)); }