Skip to content

Commit

Permalink
change public and internal functions to use int return values
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem committed Jul 17, 2020
1 parent 43c1966 commit b07731e
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 68 deletions.
23 changes: 22 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,36 @@

- The `sentry_options_set_logger` function now accepts a `userdata` parameter.
- The `name` parameter of `sentry_options_add_attachment(w)` was removed, it will now be inferred from the filename of `path`.
- The transport startup hook that is set via `sentry_transport_set_startup_func` now needs to return a `bool`, and a failure will propagate to `sentry_init`.
- The transport startup hook that is set via `sentry_transport_set_startup_func` now needs to return an `int`, and a failure will propagate to `sentry_init`.
- The return value of the transport shutdown hook set via `sentry_transport_set_shutdown_func` was also changed to return an `int`.
- Both functions should return _0_ on success, and a non-zero error code on failure, as does `sentry_init`.
- Similarly, the return value of `sentry_shutdown` was also changed to an `int`, and will return _0_ on success and a non-zero error code on unclean shutdown.

```c
// before
sentry_options_set_logger(options, my_custom_logger);
sentry_options_add_attachment(options, "some-attachment", "/path/to/some-attachment.txt");

void transport_startup(sentry_options_t *options, void*state) {
}
sentry_transport_set_startup_func(transport, transport_startup);
bool transport_shutdown(uint64_t timeout, void*state) {
return true;
}
sentry_transport_set_shutdown_func(transport, transport_shutdown);

// after
sentry_options_set_logger(options, my_custom_logger, NULL);
sentry_options_add_attachment(options, "/path/to/some-attachment.txt");

int transport_startup(sentry_options_t *options, void*state) {
return 0;
}
sentry_transport_set_startup_func(transport, transport_startup);
int transport_shutdown(uint64_t timeout, void*state) {
return 0;
}
sentry_transport_set_shutdown_func(transport, transport_shutdown);
```
**Features**:
Expand Down
1 change: 1 addition & 0 deletions examples/example.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "sentry.h"
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Expand Down
19 changes: 10 additions & 9 deletions include/sentry.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ extern "C" {

#include <inttypes.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>

/* context type dependencies */
Expand Down Expand Up @@ -561,22 +560,21 @@ SENTRY_API void sentry_transport_set_free_func(
*
* This hook is called from within `sentry_init` and will get a reference to the
* options which can be used to initialize a transports internal state.
* Returning `false` from this hook will signal failure and will bubble up to
* `sentry_init`.
* It should return `0` on success. A failure will bubble up to `sentry_init`.
*/
SENTRY_API void sentry_transport_set_startup_func(sentry_transport_t *transport,
bool (*startup_func)(const sentry_options_t *options, void *state));
int (*startup_func)(const sentry_options_t *options, void *state));

/**
* Sets the transport shutdown hook.
*
* This hook will receive a millisecond-resolution timeout; it should return
* `true` in case all the pending envelopes have been sent within the timeout,
* or `false` if the timeout was hit.
* This hook will receive a millisecond-resolution timeout.
* It should return `0` on success in case all the pending envelopes have been
* sent within the timeout, or `1` if the timeout was hit.
*/
SENTRY_API void sentry_transport_set_shutdown_func(
sentry_transport_t *transport,
bool (*shutdown_func)(uint64_t timeout, void *state));
int (*shutdown_func)(uint64_t timeout, void *state));

/**
* Generic way to free a transport.
Expand Down Expand Up @@ -885,13 +883,16 @@ SENTRY_API void sentry_options_set_system_crash_reporter_enabled(
* they cannot be modified any more.
* Depending on the configured transport and backend, this function might not be
* fully thread-safe.
* Returns 0 on success.
*/
SENTRY_API int sentry_init(sentry_options_t *options);

/**
* Shuts down the sentry client and forces transports to flush out.
*
* Returns 0 on success.
*/
SENTRY_API void sentry_shutdown(void);
SENTRY_API int sentry_shutdown(void);

/**
* Clears the internal module cache.
Expand Down
2 changes: 1 addition & 1 deletion src/backends/sentry_backend_breakpad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ sentry__breakpad_backend_callback(
return succeeded;
}

static bool
static int
sentry__breakpad_backend_startup(
sentry_backend_t *backend, const sentry_options_t *options)
{
Expand Down
8 changes: 4 additions & 4 deletions src/backends/sentry_backend_crashpad.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ sentry__crashpad_backend_shutdown(sentry_backend_t *backend)
data->db = nullptr;
}

static bool
static int
sentry__crashpad_backend_startup(
sentry_backend_t *backend, const sentry_options_t *options)
{
Expand Down Expand Up @@ -130,7 +130,7 @@ sentry__crashpad_backend_startup(
|| !sentry__path_is_file(absolute_handler_path)) {
SENTRY_WARN("unable to start crashpad backend, invalid handler_path");
sentry__path_free(absolute_handler_path);
return false;
return 1;
}

SENTRY_TRACEF("starting crashpad backend with handler "
Expand Down Expand Up @@ -191,7 +191,7 @@ sentry__crashpad_backend_startup(
// not calling `shutdown`
delete data->db;
data->db = nullptr;
return false;
return 1;
}

if (!options->system_crash_reporter_enabled) {
Expand All @@ -202,7 +202,7 @@ sentry__crashpad_backend_startup(
crashpad_info->set_system_crash_reporter_forwarding(
crashpad::TriState::kDisabled);
}
return true;
return 0;
}

static void
Expand Down
12 changes: 6 additions & 6 deletions src/backends/sentry_backend_inproc.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ invoke_signal_handler(int signum, siginfo_t *info, void *user_context)
}
}

static bool
static int
startup_inproc_backend(
sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options))
{
Expand All @@ -81,14 +81,14 @@ startup_inproc_backend(
if (sigaction(
SIGNAL_DEFINITIONS[i].signum, NULL, &g_previous_handlers[i])
== -1) {
return false;
return 1;
}
}

// install our own signal handler
g_signal_stack.ss_sp = sentry_malloc(SIGNAL_STACK_SIZE);
if (!g_signal_stack.ss_sp) {
return false;
return 1;
}
g_signal_stack.ss_size = SIGNAL_STACK_SIZE;
g_signal_stack.ss_flags = 0;
Expand All @@ -100,7 +100,7 @@ startup_inproc_backend(
for (size_t i = 0; i < SIGNAL_COUNT; ++i) {
sigaction(SIGNAL_DEFINITIONS[i].signum, &g_sigaction, NULL);
}
return true;
return 0;
}

static void
Expand Down Expand Up @@ -148,13 +148,13 @@ static const struct signal_slot SIGNAL_DEFINITIONS[SIGNAL_COUNT] = {

static LONG WINAPI handle_exception(EXCEPTION_POINTERS *);

static bool
static int
startup_inproc_backend(
sentry_backend_t *UNUSED(backend), const sentry_options_t *UNUSED(options))
{
g_previous_handler = SetUnhandledExceptionFilter(&handle_exception);
SetErrorMode(SEM_FAILCRITICALERRORS);
return true;
return 0;
}

static void
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
struct sentry_backend_s;
typedef struct sentry_backend_s {
bool (*startup_func)(
int (*startup_func)(
struct sentry_backend_s *, const sentry_options_t *options);
void (*shutdown_func)(struct sentry_backend_s *);
void (*free_func)(struct sentry_backend_s *);
Expand Down
1 change: 1 addition & 0 deletions src/sentry_boot.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,6 @@
#endif

#include <sentry.h>
#include <stdbool.h>

#endif
22 changes: 12 additions & 10 deletions src/sentry_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ sentry_init(sentry_options_t *options)
load_user_consent(options);

if (transport) {
if (!sentry__transport_startup(transport, options)) {
if (sentry__transport_startup(transport, options) != 0) {
SENTRY_WARN("failed to initialize transport");
goto fail;
}
Expand All @@ -107,7 +107,7 @@ sentry_init(sentry_options_t *options)
sentry_backend_t *backend = options->backend;
if (backend && backend->startup_func) {
SENTRY_TRACE("starting backend");
if (!backend->startup_func(backend, options)) {
if (backend->startup_func(backend, options) != 0) {
SENTRY_WARN("failed to initialize backend");
goto fail;
}
Expand Down Expand Up @@ -147,7 +147,7 @@ sentry_init(sentry_options_t *options)
return 1;
}

void
int
sentry_shutdown(void)
{
sentry_end_session();
Expand All @@ -156,21 +156,22 @@ sentry_shutdown(void)
sentry_options_t *options = g_options;
sentry__mutex_unlock(&g_options_mutex);

size_t dumped_envelopes = 0;
if (options) {
size_t dumped_envelopes = 0;
if (options->backend && options->backend->shutdown_func) {
SENTRY_TRACE("shutting down backend");
options->backend->shutdown_func(options->backend);
}
if (options->transport) {
// TODO: make this configurable
if (!sentry__transport_shutdown(
options->transport, SENTRY_DEFAULT_SHUTDOWN_TIMEOUT)) {
if (sentry__transport_shutdown(
options->transport, SENTRY_DEFAULT_SHUTDOWN_TIMEOUT)
!= 0) {
SENTRY_WARN("transport did not shut down cleanly");
}
dumped_envelopes = sentry__transport_dump_queue(
options->transport, options->run);
}
if (options->backend && options->backend->shutdown_func) {
SENTRY_TRACE("shutting down backend");
options->backend->shutdown_func(options->backend);
}
if (!dumped_envelopes) {
sentry__run_clean(options->run);
}
Expand All @@ -182,6 +183,7 @@ sentry_shutdown(void)
sentry__mutex_unlock(&g_options_mutex);
sentry__scope_cleanup();
sentry__modulefinder_cleanup();
return (int)dumped_envelopes;
}

void
Expand Down
4 changes: 3 additions & 1 deletion src/sentry_sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ worker_thread(void *data)
return 0;
}

void
int
sentry__bgworker_start(sentry_bgworker_t *bgw)
{
SENTRY_TRACE("starting background worker thread");
Expand All @@ -204,7 +204,9 @@ sentry__bgworker_start(sentry_bgworker_t *bgw)
if (sentry__thread_spawn(&bgw->thread_id, &worker_thread, bgw) != 0) {
sentry__atomic_fetch_and_add(&bgw->running, -1);
sentry__bgworker_decref(bgw);
return 1;
}
return 0;
}

static void
Expand Down
3 changes: 2 additions & 1 deletion src/sentry_sync.h
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,9 @@ void sentry__bgworker_decref(sentry_bgworker_t *bgw);

/**
* Start a new background worker thread associated with `bgw`.
* Returns 0 on success.
*/
void sentry__bgworker_start(sentry_bgworker_t *bgw);
int sentry__bgworker_start(sentry_bgworker_t *bgw);

/**
* This will try to shut down the background worker thread, with a `timeout`.
Expand Down
27 changes: 14 additions & 13 deletions src/sentry_transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@

typedef struct sentry_transport_s {
void (*send_envelope_func)(sentry_envelope_t *envelope, void *state);
bool (*startup_func)(const sentry_options_t *options, void *state);
bool (*shutdown_func)(uint64_t timeout, void *state);
int (*startup_func)(const sentry_options_t *options, void *state);
int (*shutdown_func)(uint64_t timeout, void *state);
void (*free_func)(void *state);
size_t (*dump_func)(sentry_run_t *run, void *state);
void *state;
bool was_started;
bool running;
} sentry_transport_t;

sentry_transport_t *
Expand Down Expand Up @@ -45,14 +45,14 @@ sentry_transport_set_free_func(

void
sentry_transport_set_startup_func(sentry_transport_t *transport,
bool (*startup_func)(const sentry_options_t *options, void *state))
int (*startup_func)(const sentry_options_t *options, void *state))
{
transport->startup_func = startup_func;
}

void
sentry_transport_set_shutdown_func(sentry_transport_t *transport,
bool (*shutdown_func)(uint64_t timeout, void *state))
int (*shutdown_func)(uint64_t timeout, void *state))
{
transport->shutdown_func = shutdown_func;
}
Expand All @@ -65,27 +65,28 @@ sentry__transport_send_envelope(
transport->send_envelope_func(envelope, transport->state);
}

bool
int
sentry__transport_startup(
sentry_transport_t *transport, const sentry_options_t *options)
{
if (transport->startup_func) {
SENTRY_TRACE("starting transport");
transport->was_started
= transport->startup_func(options, transport->state);
return transport->was_started;
int rv = transport->startup_func(options, transport->state);
transport->running = rv == 0;
return rv;
}
return true;
return 0;
}

bool
int
sentry__transport_shutdown(sentry_transport_t *transport, uint64_t timeout)
{
if (transport->shutdown_func && transport->was_started) {
if (transport->shutdown_func && transport->running) {
SENTRY_TRACE("shutting down transport");
transport->running = false;
return transport->shutdown_func(timeout, transport->state);
}
return true;
return 0;
}

void
Expand Down
9 changes: 6 additions & 3 deletions src/sentry_transport.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,18 @@ void sentry__transport_send_envelope(

/**
* Calls the transports startup hook.
*
* Returns 0 on success.
*/
bool sentry__transport_startup(
int sentry__transport_startup(
sentry_transport_t *transport, const sentry_options_t *options);

/**
* Instructs the transport to shut down.
*
* Returns 0 on success.
*/
bool sentry__transport_shutdown(
sentry_transport_t *transport, uint64_t timeout);
int sentry__transport_shutdown(sentry_transport_t *transport, uint64_t timeout);

/**
* This will create a new platform specific HTTP transport.
Expand Down
Loading

0 comments on commit b07731e

Please sign in to comment.