Merge pull request #734 from nicolasnoble/feedback

Addressing a first batch of feedback.

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.

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);

include/grpc/support/sync_posix.h

@@ -36,7 +36,6 @@
 
 #include <grpc/support/sync_generic.h>
 
-/* Posix variant of gpr_sync_platform.h */
 #include <pthread.h>
 
 typedef pthread_mutex_t gpr_mu;

include/grpc/support/sync_win32.h

@@ -36,7 +36,6 @@
 
 #include <grpc/support/sync_generic.h>
 
-/* Win32 variant of gpr_sync_platform.h */
 #include <windows.h>
 
 typedef struct {

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);

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);

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;
   }

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) {

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_sec = ns / GPR_NS_PER_SEC;
-    result.tv_nsec = ns - result.tv_sec * 1000000000;
+    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_sec = (-(999999999 - (ns + GPR_NS_PER_SEC)) / GPR_NS_PER_SEC) - 1;
-    result.tv_nsec = ns - result.tv_sec * 1000000000;
+    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) {
+  if (sum.tv_nsec >= GPR_NS_PER_SEC) {
-    sum.tv_nsec -= 1000000000;
+    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)) {

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<std::mutex> 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<std::mutex> lock(mu_);
         if (!have_work()) {
           cv_.wait(lock, have_work);
         }

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) {