From 65b0759653bf36e6ed71e61f57796b85ab6f5ac9 Mon Sep 17 00:00:00 2001 From: Nicolas Noble Date: Mon, 23 Feb 2015 15:28:04 -0800 Subject: [PATCH] Addressing a first batch of feedback. --- include/grpc/support/atm.h | 4 ++-- include/grpc/support/sync.h | 2 +- include/grpc/support/sync_posix.h | 1 - include/grpc/support/sync_win32.h | 1 - include/grpc/support/time.h | 2 +- src/core/support/log_win32.c | 2 +- src/core/support/string_posix.c | 2 +- src/core/support/sync.c | 4 ++-- src/core/support/time.c | 14 +++++++------- src/cpp/server/thread_pool.cc | 6 +++--- test/core/support/time_test.c | 8 ++++---- 11 files changed, 22 insertions(+), 24 deletions(-) diff --git a/include/grpc/support/atm.h b/include/grpc/support/atm.h index 0cac9bf5865..f1e30d31e80 100644 --- a/include/grpc/support/atm.h +++ b/include/grpc/support/atm.h @@ -51,12 +51,12 @@ The routines may be implemented as macros. - // Atomic operations acton an intergral_type gpr_atm that is guaranteed to + // Atomic operations act on an intergral_type gpr_atm that is guaranteed to // be the same size as a pointer. typedef gpr_intptr gpr_atm; // A memory barrier, providing both acquire and release semantics, but not - // otherwise acting no memory. + // otherwise acting on memory. void gpr_atm_full_barrier(void); // Atomically return *p, with acquire semantics. diff --git a/include/grpc/support/sync.h b/include/grpc/support/sync.h index 4437375db72..bc99317f3c9 100644 --- a/include/grpc/support/sync.h +++ b/include/grpc/support/sync.h @@ -206,7 +206,7 @@ void *gpr_event_cancellable_wait(gpr_event *ev, gpr_timespec abs_deadline, /* --- Reference counting --- - These calls act on the type gpr_refcount. It requires no desctruction. */ + These calls act on the type gpr_refcount. It requires no destruction. */ /* Initialize *r to value n. */ void gpr_ref_init(gpr_refcount *r, int n); diff --git a/include/grpc/support/sync_posix.h b/include/grpc/support/sync_posix.h index 413226a9e8f..8ba2c5b8920 100644 --- a/include/grpc/support/sync_posix.h +++ b/include/grpc/support/sync_posix.h @@ -36,7 +36,6 @@ #include -/* Posix variant of gpr_sync_platform.h */ #include typedef pthread_mutex_t gpr_mu; diff --git a/include/grpc/support/sync_win32.h b/include/grpc/support/sync_win32.h index 5a48b52a2dc..13823b8ee3d 100644 --- a/include/grpc/support/sync_win32.h +++ b/include/grpc/support/sync_win32.h @@ -36,7 +36,6 @@ #include -/* Win32 variant of gpr_sync_platform.h */ #include typedef struct { diff --git a/include/grpc/support/time.h b/include/grpc/support/time.h index ebc18c91e94..150b7ac8c53 100644 --- a/include/grpc/support/time.h +++ b/include/grpc/support/time.h @@ -76,7 +76,7 @@ gpr_timespec gpr_time_min(gpr_timespec a, gpr_timespec b); gpr_timespec gpr_time_add(gpr_timespec a, gpr_timespec b); gpr_timespec gpr_time_sub(gpr_timespec a, gpr_timespec b); -/* Return a timespec representing a given number of microseconds. LONG_MIN is +/* Return a timespec representing a given number of time units. LONG_MIN is interpreted as gpr_inf_past, and LONG_MAX as gpr_inf_future. */ gpr_timespec gpr_time_from_micros(long x); gpr_timespec gpr_time_from_nanos(long x); diff --git a/src/core/support/log_win32.c b/src/core/support/log_win32.c index cff130ae18d..720dc141f51 100644 --- a/src/core/support/log_win32.c +++ b/src/core/support/log_win32.c @@ -90,7 +90,7 @@ void gpr_default_log(gpr_log_func_args *args) { strcpy(time_buffer, "error:strftime"); } - fprintf(stderr, "%s%s.%09u %5u %s:%d: %s\n", + fprintf(stderr, "%s%s.%09u %5u %s:%d] %s\n", gpr_log_severity_string(args->severity), time_buffer, (int)(now.tv_nsec), GetCurrentThreadId(), args->file, args->line, args->message); diff --git a/src/core/support/string_posix.c b/src/core/support/string_posix.c index 8a678b3103d..25c333db4ed 100644 --- a/src/core/support/string_posix.c +++ b/src/core/support/string_posix.c @@ -51,7 +51,7 @@ int gpr_asprintf(char **strp, const char *format, ...) { va_start(args, format); ret = vsnprintf(buf, sizeof(buf), format, args); va_end(args); - if (!(0 <= ret)) { + if (ret < 0) { *strp = NULL; return -1; } diff --git a/src/core/support/sync.c b/src/core/support/sync.c index 1a5cf57c4f2..ccfe1e25f45 100644 --- a/src/core/support/sync.c +++ b/src/core/support/sync.c @@ -41,7 +41,7 @@ Should be a prime. */ enum { event_sync_partitions = 31 }; -/* Event are partitioned by address to avoid lock contention. */ +/* Events are partitioned by address to avoid lock contention. */ static struct sync_array_s { gpr_mu mu; gpr_cv cv; @@ -71,10 +71,10 @@ void gpr_event_set(gpr_event *ev, void *value) { struct sync_array_s *s = hash(ev); gpr_mu_lock(&s->mu); GPR_ASSERT(gpr_atm_acq_load(&ev->state) == 0); - GPR_ASSERT(value != NULL); gpr_atm_rel_store(&ev->state, (gpr_atm)value); gpr_cv_broadcast(&s->cv); gpr_mu_unlock(&s->mu); + GPR_ASSERT(value != NULL); } void *gpr_event_get(gpr_event *ev) { diff --git a/src/core/support/time.c b/src/core/support/time.c index 67f76656502..7dbf95059f5 100644 --- a/src/core/support/time.c +++ b/src/core/support/time.c @@ -85,12 +85,12 @@ gpr_timespec gpr_time_from_nanos(long ns) { } else if (ns == LONG_MIN) { result = gpr_inf_past; } else if (ns >= 0) { - result.tv_sec = ns / 1000000000; - result.tv_nsec = ns - result.tv_sec * 1000000000; + result.tv_sec = ns / GPR_NS_PER_SEC; + result.tv_nsec = ns - result.tv_sec * GPR_NS_PER_SEC; } else { /* Calculation carefully formulated to avoid any possible under/overflow. */ - result.tv_sec = (-(999999999 - (ns + 1000000000)) / 1000000000) - 1; - result.tv_nsec = ns - result.tv_sec * 1000000000; + result.tv_sec = (-(999999999 - (ns + GPR_NS_PER_SEC)) / GPR_NS_PER_SEC) - 1; + result.tv_nsec = ns - result.tv_sec * GPR_NS_PER_SEC; } return result; } @@ -172,8 +172,8 @@ gpr_timespec gpr_time_add(gpr_timespec a, gpr_timespec b) { gpr_timespec sum; int inc = 0; sum.tv_nsec = a.tv_nsec + b.tv_nsec; - if (sum.tv_nsec >= 1000000000) { - sum.tv_nsec -= 1000000000; + if (sum.tv_nsec >= GPR_NS_PER_SEC) { + sum.tv_nsec -= GPR_NS_PER_SEC; inc++; } if (a.tv_sec == TYPE_MAX(time_t) || a.tv_sec == TYPE_MIN(time_t)) { @@ -200,7 +200,7 @@ gpr_timespec gpr_time_sub(gpr_timespec a, gpr_timespec b) { int dec = 0; diff.tv_nsec = a.tv_nsec - b.tv_nsec; if (diff.tv_nsec < 0) { - diff.tv_nsec += 1000000000; + diff.tv_nsec += GPR_NS_PER_SEC; dec++; } if (a.tv_sec == TYPE_MAX(time_t) || a.tv_sec == TYPE_MIN(time_t)) { diff --git a/src/cpp/server/thread_pool.cc b/src/cpp/server/thread_pool.cc index 1ca98129d3e..fa11ddd04c7 100644 --- a/src/cpp/server/thread_pool.cc +++ b/src/cpp/server/thread_pool.cc @@ -37,11 +37,11 @@ namespace grpc { ThreadPool::ThreadPool(int num_threads) { for (int i = 0; i < num_threads; i++) { - threads_.push_back(std::thread([=]() { + threads_.push_back(std::thread([this]() { for (;;) { - std::unique_lock lock(mu_); // Wait until work is available or we are shutting down. - auto have_work = [=]() { return shutdown_ || !callbacks_.empty(); }; + auto have_work = [this]() { return shutdown_ || !callbacks_.empty(); }; + std::unique_lock lock(mu_); if (!have_work()) { cv_.wait(lock, have_work); } diff --git a/test/core/support/time_test.c b/test/core/support/time_test.c index 2741e17f95f..c1dce777b09 100644 --- a/test/core/support/time_test.c +++ b/test/core/support/time_test.c @@ -81,7 +81,7 @@ static void ts_to_s(gpr_timespec t, void *arg) { if (t.tv_sec < 0 && t.tv_nsec != 0) { t.tv_sec++; - t.tv_nsec = 1000000000 - t.tv_nsec; + t.tv_nsec = GPR_NS_PER_SEC - t.tv_nsec; } i_to_s(t.tv_sec, 10, 0, writer, arg); (*writer)(arg, ".", 1); @@ -127,15 +127,15 @@ static void test_values(void) { /* Test possible overflow in conversion of -ve values. */ x = gpr_time_from_micros(-(LONG_MAX - 999997)); GPR_ASSERT(x.tv_sec < 0); - GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < 1000000000); + GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < GPR_NS_PER_SEC); x = gpr_time_from_nanos(-(LONG_MAX - 999999997)); GPR_ASSERT(x.tv_sec < 0); - GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < 1000000000); + GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < GPR_NS_PER_SEC); x = gpr_time_from_millis(-(LONG_MAX - 997)); GPR_ASSERT(x.tv_sec < 0); - GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < 1000000000); + GPR_ASSERT(x.tv_nsec >= 0 && x.tv_nsec < GPR_NS_PER_SEC); /* Test general -ve values. */ for (i = -1; i > -1000 * 1000 * 1000; i *= 7) {