Skip to content

Commit

Permalink
[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_AS…
Browse files Browse the repository at this point in the history
…SERT (#36468)

[grpc][Gpr_To_Absl_Logging] Migrating from gpr to absl logging GPR_ASSERT
Replacing GPR_ASSERT with absl CHECK.
These changes have been made using string replacement and regex.
Will not be replacing all instances of CHECK with CHECK_EQ , CHECK_NE etc because there are too many callsites. Only ones which are doable using very simple regex with least chance of failure will be replaced.
Given that we have 5000+ instances of GPR_ASSERT to edit, Doing it manually is too much work for both the author and reviewer.

<!--

If you know who should review your pull request, please assign it to that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the appropriate
lang label.

-->

Closes #36468

COPYBARA_INTEGRATE_REVIEW=#36468 from tanvi-jagtap:tjagtap_src_core_lib_gprpp f7ed931
PiperOrigin-RevId: 630978821
  • Loading branch information
tanvi-jagtap authored and Copybara-Service committed May 6, 2024
1 parent fb72f1d commit a20f020
Show file tree
Hide file tree
Showing 32 changed files with 164 additions and 102 deletions.
4 changes: 4 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ grpc_cc_library(
"absl/base:log_severity",
"absl/functional:any_invocable",
"absl/log",
"absl/log:check",
"absl/log:globals",
"absl/memory",
"absl/random",
"absl/status",
Expand Down Expand Up @@ -2962,6 +2964,7 @@ grpc_cc_library(
external_deps = [
"absl/base:core_headers",
"absl/container:inlined_vector",
"absl/log:check",
],
language = "c++",
visibility = ["@grpc:client_channel"],
Expand Down Expand Up @@ -4836,6 +4839,7 @@ grpc_cc_library(
"//src/core:lib/gpr/subprocess.h",
],
external_deps = [
"absl/log:check",
"absl/strings",
"absl/types:span",
],
Expand Down
11 changes: 6 additions & 5 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions build_autogenerated.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gRPC-C++.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions gRPC-Core.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ grpc_cc_library(
grpc_cc_library(
name = "chunked_vector",
hdrs = ["lib/gprpp/chunked_vector.h"],
external_deps = [
"absl/log:check",
],
deps = [
"arena",
"gpr_manual_constructor",
Expand All @@ -352,6 +355,7 @@ grpc_cc_library(
"lib/gprpp/status_helper.h",
],
external_deps = [
"absl/log:check",
"absl/status",
"absl/strings",
"absl/strings:cord",
Expand Down Expand Up @@ -1648,6 +1652,7 @@ grpc_cc_library(
"lib/gprpp/time.h",
],
external_deps = [
"absl/log:check",
"absl/strings:str_format",
"absl/types:optional",
],
Expand Down Expand Up @@ -3085,6 +3090,9 @@ grpc_cc_library(
hdrs = [
"lib/gprpp/single_set_ptr.h",
],
external_deps = [
"absl/log:check",
],
language = "c++",
deps = ["//:gpr"],
)
Expand Down
4 changes: 3 additions & 1 deletion src/core/lib/gpr/alloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include <stdlib.h>
#include <string.h>

#include "absl/log/check.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>

Expand Down Expand Up @@ -59,7 +61,7 @@ void* gpr_realloc(void* p, size_t size) {
}

void* gpr_malloc_aligned(size_t size, size_t alignment) {
GPR_ASSERT(((alignment - 1) & alignment) == 0); // Must be power of 2.
CHECK_EQ(((alignment - 1) & alignment), 0u); // Must be power of 2.
size_t extra = alignment - 1 + sizeof(void*);
void* p = gpr_malloc(size + extra);
void** ret = reinterpret_cast<void**>(
Expand Down
48 changes: 25 additions & 23 deletions src/core/lib/gpr/posix/sync.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <errno.h>
#include <time.h>

#include "absl/log/check.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/sync.h>
Expand All @@ -33,36 +35,36 @@

void gpr_mu_init(gpr_mu* mu) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_mutex_init(&mu->mutex, nullptr) == 0);
CHECK_EQ(pthread_mutex_init(&mu->mutex, nullptr), 0);
mu->leak_checker = static_cast<int*>(malloc(sizeof(*mu->leak_checker)));
GPR_ASSERT(mu->leak_checker != nullptr);
CHECK_NE(mu->leak_checker, nullptr);
#else
GPR_ASSERT(pthread_mutex_init(mu, nullptr) == 0);
CHECK_EQ(pthread_mutex_init(mu, nullptr), 0);
#endif
}

void gpr_mu_destroy(gpr_mu* mu) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_mutex_destroy(&mu->mutex) == 0);
CHECK_EQ(pthread_mutex_destroy(&mu->mutex), 0);
free(mu->leak_checker);
#else
GPR_ASSERT(pthread_mutex_destroy(mu) == 0);
CHECK_EQ(pthread_mutex_destroy(mu), 0);
#endif
}

void gpr_mu_lock(gpr_mu* mu) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_mutex_lock(&mu->mutex) == 0);
CHECK_EQ(pthread_mutex_lock(&mu->mutex), 0);
#else
GPR_ASSERT(pthread_mutex_lock(mu) == 0);
CHECK_EQ(pthread_mutex_lock(mu), 0);
#endif
}

void gpr_mu_unlock(gpr_mu* mu) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_mutex_unlock(&mu->mutex) == 0);
CHECK_EQ(pthread_mutex_unlock(&mu->mutex), 0);
#else
GPR_ASSERT(pthread_mutex_unlock(mu) == 0);
CHECK_EQ(pthread_mutex_unlock(mu), 0);
#endif
}

Expand All @@ -73,34 +75,34 @@ int gpr_mu_trylock(gpr_mu* mu) {
#else
err = pthread_mutex_trylock(mu);
#endif
GPR_ASSERT(err == 0 || err == EBUSY);
CHECK(err == 0 || err == EBUSY);
return err == 0;
}

//----------------------------------------

void gpr_cv_init(gpr_cv* cv) {
pthread_condattr_t attr;
GPR_ASSERT(pthread_condattr_init(&attr) == 0);
CHECK_EQ(pthread_condattr_init(&attr), 0);
#if GPR_LINUX
GPR_ASSERT(pthread_condattr_setclock(&attr, CLOCK_MONOTONIC) == 0);
CHECK_EQ(pthread_condattr_setclock(&attr, CLOCK_MONOTONIC), 0);
#endif // GPR_LINUX

#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_cond_init(&cv->cond_var, &attr) == 0);
CHECK_EQ(pthread_cond_init(&cv->cond_var, &attr), 0);
cv->leak_checker = static_cast<int*>(malloc(sizeof(*cv->leak_checker)));
GPR_ASSERT(cv->leak_checker != nullptr);
CHECK_NE(cv->leak_checker, nullptr);
#else
GPR_ASSERT(pthread_cond_init(cv, &attr) == 0);
CHECK_EQ(pthread_cond_init(cv, &attr), 0);
#endif
}

void gpr_cv_destroy(gpr_cv* cv) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_cond_destroy(&cv->cond_var) == 0);
CHECK_EQ(pthread_cond_destroy(&cv->cond_var), 0);
free(cv->leak_checker);
#else
GPR_ASSERT(pthread_cond_destroy(cv) == 0);
CHECK_EQ(pthread_cond_destroy(cv), 0);
#endif
}

Expand Down Expand Up @@ -129,30 +131,30 @@ int gpr_cv_wait(gpr_cv* cv, gpr_mu* mu, gpr_timespec abs_deadline) {
err = pthread_cond_timedwait(cv, mu, &abs_deadline_ts);
#endif
}
GPR_ASSERT(err == 0 || err == ETIMEDOUT || err == EAGAIN);
CHECK(err == 0 || err == ETIMEDOUT || err == EAGAIN);
return err == ETIMEDOUT;
}

void gpr_cv_signal(gpr_cv* cv) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_cond_signal(&cv->cond_var) == 0);
CHECK_EQ(pthread_cond_signal(&cv->cond_var), 0);
#else
GPR_ASSERT(pthread_cond_signal(cv) == 0);
CHECK_EQ(pthread_cond_signal(cv), 0);
#endif
}

void gpr_cv_broadcast(gpr_cv* cv) {
#ifdef GRPC_ASAN_ENABLED
GPR_ASSERT(pthread_cond_broadcast(&cv->cond_var) == 0);
CHECK_EQ(pthread_cond_broadcast(&cv->cond_var), 0);
#else
GPR_ASSERT(pthread_cond_broadcast(cv) == 0);
CHECK_EQ(pthread_cond_broadcast(cv), 0);
#endif
}

//----------------------------------------

void gpr_once_init(gpr_once* once, void (*init_function)(void)) {
GPR_ASSERT(pthread_once(once, init_function) == 0);
CHECK_EQ(pthread_once(once, init_function), 0);
}

#endif // defined(GPR_POSIX_SYNC) && !defined(GPR_ABSEIL_SYNC) &&
Expand Down
15 changes: 9 additions & 6 deletions src/core/lib/gpr/posix/time.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
#ifdef __linux__
#include <sys/syscall.h>
#endif
#include "absl/log/check.h"

#include <grpc/support/atm.h>
#include <grpc/support/log.h>
#include <grpc/support/time.h>
Expand All @@ -38,7 +40,8 @@ static struct timespec timespec_from_gpr(gpr_timespec gts) {
struct timespec rv;
if (sizeof(time_t) < sizeof(int64_t)) {
// fine to assert, as this is only used in gpr_sleep_until
GPR_ASSERT(gts.tv_sec <= INT32_MAX && gts.tv_sec >= INT32_MIN);
CHECK(gts.tv_sec <= INT32_MAX);
CHECK(gts.tv_sec >= INT32_MIN);
}
rv.tv_sec = static_cast<time_t>(gts.tv_sec);
rv.tv_nsec = gts.tv_nsec;
Expand Down Expand Up @@ -67,7 +70,7 @@ void gpr_time_init(void) { gpr_precise_clock_init(); }

static gpr_timespec now_impl(gpr_clock_type clock_type) {
struct timespec now;
GPR_ASSERT(clock_type != GPR_TIMESPAN);
CHECK(clock_type != GPR_TIMESPAN);
if (clock_type == GPR_CLOCK_PRECISE) {
gpr_timespec ret;
gpr_precise_clock_now(&ret);
Expand All @@ -87,12 +90,12 @@ gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type) = now_impl;

gpr_timespec gpr_now(gpr_clock_type clock_type) {
// validate clock type
GPR_ASSERT(clock_type == GPR_CLOCK_MONOTONIC ||
clock_type == GPR_CLOCK_REALTIME ||
clock_type == GPR_CLOCK_PRECISE);
CHECK(clock_type == GPR_CLOCK_MONOTONIC || clock_type == GPR_CLOCK_REALTIME ||
clock_type == GPR_CLOCK_PRECISE);
gpr_timespec ts = gpr_now_impl(clock_type);
// tv_nsecs must be in the range [0, 1e9).
GPR_ASSERT(ts.tv_nsec >= 0 && ts.tv_nsec < 1e9);
CHECK(ts.tv_nsec >= 0);
CHECK(ts.tv_nsec < 1e9);
return ts;
}

Expand Down
4 changes: 3 additions & 1 deletion src/core/lib/gpr/posix/tmpfile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include <string.h>
#include <unistd.h>

#include "absl/log/check.h"

#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
Expand All @@ -42,7 +44,7 @@ FILE* gpr_tmpfile(const char* prefix, char** tmp_filename) {
if (tmp_filename != nullptr) *tmp_filename = nullptr;

gpr_asprintf(&filename_template, "/tmp/%s_XXXXXX", prefix);
GPR_ASSERT(filename_template != nullptr);
CHECK_NE(filename_template, nullptr);

fd = mkstemp(filename_template);
if (fd == -1) {
Expand Down
9 changes: 5 additions & 4 deletions src/core/lib/gpr/subprocess_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include <iostream>

#include "absl/log/check.h"
#include "absl/strings/substitute.h"

#include <grpc/support/alloc.h>
Expand Down Expand Up @@ -81,8 +82,8 @@ gpr_subprocess* gpr_subprocess_create_with_envp(int argc, const char** argv,
int stdout_pipe[2];
int p0 = pipe(stdin_pipe);
int p1 = pipe(stdout_pipe);
GPR_ASSERT(p0 != -1);
GPR_ASSERT(p1 != -1);
CHECK_NE(p0, -1);
CHECK_NE(p1, -1);
pid = fork();
if (pid == -1) {
return nullptr;
Expand Down Expand Up @@ -145,7 +146,7 @@ bool gpr_subprocess_communicate(gpr_subprocess* p, std::string& input_data,
continue;
} else {
std::cerr << "select: " << strerror(errno) << std::endl;
GPR_ASSERT(0);
CHECK(0);
}
}

Expand Down Expand Up @@ -192,7 +193,7 @@ bool gpr_subprocess_communicate(gpr_subprocess* p, std::string& input_data,
while (waitpid(p->pid, &status, 0) == -1) {
if (errno != EINTR) {
std::cerr << "waitpid: " << strerror(errno) << std::endl;
GPR_ASSERT(0);
CHECK(0);
}
}

Expand Down

0 comments on commit a20f020

Please sign in to comment.