Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
camshaft committed Sep 21, 2023
1 parent 5430519 commit 3895794
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 23 deletions.
4 changes: 2 additions & 2 deletions api/s2n.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ extern "C" {
* explaining the error in English by calling s2n_strerror(s2n_errno, "EN").
* A string containing human readable error name; can be generated with `s2n_strerror_name`.
* A string containing internal debug information, including filename and line number, can be generated with `s2n_strerror_debug`.
* A string containing only the filename and line number can be generated with `s2n_strerror_file_line`.
* A string containing only the filename and line number can be generated with `s2n_strerror_source`.
* This string is useful to include when reporting issues to the s2n-tls development team.
*
* @warning To avoid possible confusion, s2n_errno should be cleared after processing an error: `s2n_errno = S2N_ERR_T_OK`
Expand Down Expand Up @@ -414,7 +414,7 @@ S2N_API extern const char *s2n_strerror_name(int error);
* @param error The error code to explain. Usually this is s2n_errno
* @returns The error string.
*/
S2N_API extern const char *s2n_strerror_file_line(int error);
S2N_API extern const char *s2n_strerror_source(int error);

/**
* Opaque stack trace structure.
Expand Down
2 changes: 1 addition & 1 deletion codebuild/bin/grep_simple_mistakes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ FAILED=0
# Grep for any instances of raw memcpy() function. s2n code should instead be
# using one of the *_ENSURE_MEMCPY macros.
#############################################
S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/pq-crypto/*" -not -path "*/s2n_ensure.[ch]")
S2N_FILES_ASSERT_NOT_USING_MEMCPY=$(find "$PWD" -type f -name "s2n*.[ch]" -not -path "*/tests/*" -not -path "*/pq-crypto/*")
for file in $S2N_FILES_ASSERT_NOT_USING_MEMCPY; do
RESULT_NUM_LINES=`grep 'memcpy(' $file | wc -l`
if [ "${RESULT_NUM_LINES}" != 0 ]; then
Expand Down
8 changes: 4 additions & 4 deletions error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
#endif

__thread int s2n_errno;
__thread struct s2n_debug_info _s2n_debug_info = { .debug_str = "", .file_line = "" };
__thread struct s2n_debug_info _s2n_debug_info = { .debug_str = "", .source = "" };

/**
* Returns the address of the thread-local `s2n_errno` variable
Expand Down Expand Up @@ -381,14 +381,14 @@ const char *s2n_strerror_debug(int error, const char *lang)
return _s2n_debug_info.debug_str;
}

const char *s2n_strerror_file_line(int error)
const char *s2n_strerror_source(int error)
{
/* No error, just return an empty string */
if (error == S2N_ERR_OK) {
return "";
}

return _s2n_debug_info.file_line;
return _s2n_debug_info.source;
}

int s2n_error_get_type(int error)
Expand All @@ -413,7 +413,7 @@ int s2n_stack_traces_enabled_set(bool newval)
void s2n_debug_info_reset(void)
{
_s2n_debug_info.debug_str = "";
_s2n_debug_info.file_line = "";
_s2n_debug_info.source = "";
}

#ifdef S2N_STACKTRACE
Expand Down
26 changes: 11 additions & 15 deletions error/s2n_errno.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ typedef enum {
#define S2N_DEBUG_STR_LEN 128

struct s2n_debug_info {
const char* debug_str;
const char* file_line;
const char *debug_str;
const char *source;
};

extern __thread struct s2n_debug_info _s2n_debug_info;
Expand All @@ -345,16 +345,12 @@ extern __thread struct s2n_debug_info _s2n_debug_info;
#define _S2N_DEBUG_LINE_PREFIX "Error encountered in "
#define _S2N_DEBUG_LINE _S2N_DEBUG_LINE_PREFIX __FILE__ ":" STRING__LINE__

#define _S2N_DEBUG_INFO ((struct s2n_debug_info){ \
.debug_str = _S2N_DEBUG_LINE, \
.file_line = _S2N_EXTRACT_BASENAME(_S2N_DEBUG_LINE) })

#define _S2N_ERROR(x) \
do { \
_s2n_debug_info.debug_str = _S2N_DEBUG_LINE; \
_s2n_debug_info.file_line = _S2N_EXTRACT_BASENAME(_s2n_debug_info.debug_str); \
s2n_errno = (x); \
s2n_calculate_stacktrace(); \
#define _S2N_ERROR(x) \
do { \
_s2n_debug_info.debug_str = _S2N_DEBUG_LINE; \
_s2n_debug_info.source = _S2N_EXTRACT_BASENAME(_s2n_debug_info.debug_str); \
s2n_errno = (x); \
s2n_calculate_stacktrace(); \
} while (0)
#define S2N_ERROR_PRESERVE_ERRNO() \
do { \
Expand Down Expand Up @@ -386,15 +382,15 @@ extern __thread struct s2n_debug_info _s2n_debug_info;

/** Calculate and print stacktraces */
struct s2n_stacktrace {
char** trace;
char **trace;
int trace_size;
};

bool s2n_stack_traces_enabled();
int s2n_stack_traces_enabled_set(bool newval);

int s2n_calculate_stacktrace(void);
int s2n_print_stacktrace(FILE* fptr);
int s2n_print_stacktrace(FILE *fptr);
int s2n_free_stacktrace(void);
int s2n_get_stacktrace(struct s2n_stacktrace* trace);
int s2n_get_stacktrace(struct s2n_stacktrace *trace);
void s2n_debug_info_reset(void);
2 changes: 1 addition & 1 deletion tests/unit/s2n_error_lookup_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ int main(void)

/* Ensure the file/line information is returned as expected */
s2n_result_ignore(test_function(false));
EXPECT_EQUAL(strcmp(s2n_strerror_file_line(S2N_ERR_SAFETY), "s2n_error_lookup_test.c:25"), 0);
EXPECT_EQUAL(strcmp(s2n_strerror_source(S2N_ERR_SAFETY), "s2n_error_lookup_test.c:25"), 0);

/* constructs a debug string from a path */
#define EXAMPLE_DEBUG_STR(path) (_S2N_EXTRACT_BASENAME(_S2N_DEBUG_LINE_PREFIX path))
Expand Down

0 comments on commit 3895794

Please sign in to comment.