Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging #127

Merged
merged 7 commits into from
Dec 7, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions include/rcutils/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,16 +196,14 @@ rcutils_logging_severity_level_from_string(
* \param severity The severity level
* \param name The name of the logger
* \param timestamp The timestamp
* \param format The format string
* \param args The variable argument list
* \param log_str The string to be logged
*/
typedef void (* rcutils_logging_output_handler_t)(
const rcutils_log_location_t *, // location
int, // severity
const char *, // name
rcutils_time_point_value_t, // timestamp
const char *, // format
va_list * // args
const char * // log_str
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the logic behind this change? (sorry if you explained it somewhere else)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched to this for two reasons. 1) I wanted to make sure the log message was consistent for every output handler. 2) I wanted to reduce the amount of redundancy in both the runtime execution of formatting logs and in the code. From what I saw, there wasn't an easy way to provide named key replacement during log formatting in libraries such as log4cxx. This would require external libraries written on top of those to either reimplement the code that replaces those keywords or restructure/move the existing code in rcutils that does that formatting/replacement so that it could be used by the external libraries.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I remember now why this is like this. The reason was so that that the output handler being used could decide how to do the formatting, specifically if they wanted to use a fixed sized string for the destination and truncate if too long (avoiding memory allocations) or if they wanted to use dynamic memory and something like snprintf.

For passing to something like log4cxx you can just vsnprintf the format string and arg list into a log string right before passing it to log4cxx. That means redundant code (every output handler needs to do this), but it also means they have control over how the formatting happens.

Given this, I think we should roll back on this specific change. Sorry for being back and forth about it.

);

/// The function pointer of the current output handler.
Expand Down Expand Up @@ -418,7 +416,8 @@ int rcutils_logging_get_logger_effective_level(const char * name);
* <hr>
* Attribute | Adherence
* ------------------ | -------------
* Allocates Memory | No
* Allocates Memory | No, for formatted outputs <= 1023 characters
* | Yes, for formatted outputs >= 1024 characters
* Thread-Safe | No
* Uses Atomics | No
* Lock-Free | Yes
Expand Down Expand Up @@ -449,8 +448,7 @@ void rcutils_log(
* <hr>
* Attribute | Adherence
* ------------------ | -------------
* Allocates Memory | No, for formatted outputs <= 1023 characters
* | Yes, for formatted outputs >= 1024 characters
* Allocates Memory | No
* Thread-Safe | Yes, if the underlying *printf functions are
* Uses Atomics | No
* Lock-Free | Yes
Expand All @@ -459,14 +457,13 @@ void rcutils_log(
* \param severity The severity level
* \param name The name of the logger, must be null terminated c string
* \param timestamp The timestamp for when the log message was made
* \param format The format string for the message contents
* \param args The variable argument list for the message format string
* \param log_str The string to be logged
*/
RCUTILS_PUBLIC
void rcutils_logging_console_output_handler(
const rcutils_log_location_t * location,
int severity, const char * name, rcutils_time_point_value_t timestamp,
const char * format, va_list * args);
const char * log_str);

// Provide the compiler with branch prediction information
#ifndef _WIN32
Expand Down
1 change: 1 addition & 0 deletions include/rcutils/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ extern "C"

#define RCUTILS_STRINGIFY_IMPL(x) #x
#define RCUTILS_STRINGIFY(x) RCUTILS_STRINGIFY_IMPL(x)
#define RCUTILS_UNUSED(x) (void)(x)

#ifdef __cplusplus
}
Expand Down
127 changes: 126 additions & 1 deletion include/rcutils/types/char_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,23 @@ extern "C"
{
#endif

#include <stdarg.h>
nburek marked this conversation as resolved.
Show resolved Hide resolved

#include "rcutils/allocator.h"
#include "rcutils/types/rcutils_ret.h"
#include "rcutils/visibility_control.h"

typedef struct RCUTILS_PUBLIC_TYPE rcutils_char_array_t
{
char * buffer;

/**
* if this is true, we may safely free/realloc the buffer as needed;
* otherwise we will leave the buffer alone and alloc new memory if
* more space is needed
*/
bool owns_buffer;
nburek marked this conversation as resolved.
Show resolved Hide resolved

size_t buffer_length;
size_t buffer_capacity;
rcutils_allocator_t allocator;
Expand Down Expand Up @@ -66,9 +76,11 @@ rcutils_char_array_init(

/// Finalize a char array struct.
/**
* Cleans up and deallocates any resources used in a rcutils_char_array_t.
* Cleans up and deallocates any resources owned by rcutils_char_array_t.
* The array passed to this function needs to have been initialized by
* rcutils_char_array_init().
* If .owns_buffer is false, this function has no effect because that
* implies that the char_array does not own the internal buffer.
* Passing an uninitialized instance to this function leads to undefined
* behavior.
*
Expand All @@ -89,6 +101,11 @@ rcutils_char_array_fini(rcutils_char_array_t * char_array);
* truncated.
* Be aware, that this will deallocate the memory and therefore invalidates any
* pointers to this storage.
* If the new size is larger, new memory is getting allocated and the existing
* content is copied over.
* Note that if the array doesn't own the current buffer the function just
* allocates a new block of memory and copies the contents of the old buffer
* instead of resizing the existing buffer.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being resized
* \param new_size the new size of the internal buffer
Expand All @@ -102,6 +119,114 @@ RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_resize(rcutils_char_array_t * char_array, size_t new_size);

/// Expand the internal buffer of the char array.
/**
* This function is equivalent to `rcutils_char_array_resize` except that it resizes
* the internal buffer only when it is not big enough.
* If the buffer is already big enough for `new_size`, it returns `RCUTILS_RET_OK` without
* doing anything.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being resized
* \param new_size the new size of the internal buffer
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_expand_as_needed(rcutils_char_array_t * char_array, size_t new_size);

/// Produce output according to format and args.
/**
* This function is equivalent to `vsprintf(char_array->buffer, format, args)`
* except that the buffer grows as needed so a user doesn't have to deal with
* memory management.
* The `va_list args` will be cloned before being used, so a user can safely
* use it again after calling this function.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being written to
* \param format the format string used by the underlying `vsnprintf`
* \param args the `va_list` used by the underlying `vsnprintf`
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_vsprintf(rcutils_char_array_t * char_array, const char * format, va_list args);

/// Append a string (or part of it) to the string in buffer.
/**
* This function treats the internal buffer as a string and appends the src string to it.
* If src is longer than n, n bytes will be used and an extra null byte will be appended.
* It is virtually equivalent to `strncat(char_array->buffer, src, n)` except that the buffer
* grows as needed so a user doesn't have to deal with memory management.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being appended to
* \param src the string to be appended to the end of the string in buffer
* \param n it uses at most n bytes from the src string
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_strncat(rcutils_char_array_t * char_array, const char * src, size_t n);

/// Append a string to the string in buffer.
/**
* This function treats the internal buffer as a string and appends the src string to it.
* It is virtually equivalent to `strcat(char_array->buffer, src)` except that the buffer
* grows as needed. That is to say, a user can safely use it without doing calculation or
* checks on the sizes of the src and buffer.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being appended to
* \param src the string to be appended to the end of the string in buffer
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_strcat(rcutils_char_array_t * char_array, const char * src);

/// Copy memory to buffer.
/**
* This function is equivalent to `memcpy(char_array->buffer, src, n)` except that the buffer
* grows as needed so a user doesn't have to worry about overflow.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being resized
* \param src the memory to be copied from
* \param n a total of n bytes will be copied
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_memcpy(rcutils_char_array_t * char_array, const char * src, size_t n);

/// Copy a string to buffer.
/**
* This function is equivalent to `strcpy(char_array->buffer, src)` except that the buffer
* grows as needed so that `src` will fit without overflow.
*
* \param char_array pointer to the instance of rcutils_char_array_t which is being copied to
* \param src the string to be copied from
* \return `RCUTILS_RET_OK` if successful, or
* \return `RCUTILS_RET_BAD_ALLOC` if memory allocation failed, or
* \return `RCUTILS_RET_ERROR` if an unexpected error occurs
*/
RCUTILS_PUBLIC
RCUTILS_WARN_UNUSED
rcutils_ret_t
rcutils_char_array_strcpy(rcutils_char_array_t * char_array, const char * src);

#if __cplusplus
}
#endif
Expand Down
8 changes: 4 additions & 4 deletions src/allocator.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,28 @@
static void *
__default_allocate(size_t size, void * state)
{
(void)state; // unused
RCUTILS_UNUSED(state);
nburek marked this conversation as resolved.
Show resolved Hide resolved
return malloc(size);
}

static void
__default_deallocate(void * pointer, void * state)
{
(void)state; // unused
RCUTILS_UNUSED(state);
free(pointer);
}

static void *
__default_reallocate(void * pointer, size_t size, void * state)
{
(void)state; // unused
RCUTILS_UNUSED(state);
return realloc(pointer, size);
}

static void *
__default_zero_allocate(size_t number_of_elements, size_t size_of_element, void * state)
{
(void)state; // unused
RCUTILS_UNUSED(state);
return calloc(number_of_elements, size_of_element);
}

Expand Down
Loading