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

WIP: add sml_error function pointer, allow user to intercept error messages #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 deletions sml/include/sml/sml_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ int sml_buf_optional_is_skipped(sml_buffer *buf);
// Prints arbitrarily byte string to stdout with printf
void hexdump(unsigned char *buffer, size_t buffer_len);

// Allow user to intercept error messages
extern void (*sml_error)(const char *format, ...)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this forces anybody using this to call va_* and vaprintf himself. (see the example in sml_error_default() below)
i wonder if this is any good,
or if we should maybe rather have an intermediate function and pass out the completely formatted string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mbehr1:any idea on this one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

might also pass more details, like a loglevel (warning/error).

// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
__attribute__((format(printf, 1, 2)));

// default sml_error function, prints message to stderr as before
void sml_error_default(const char *format, ...)
// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
__attribute__((format(printf, 1, 2)));

#ifdef __cplusplus
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion sml/src/sml_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ sml_file *sml_file_parse(unsigned char *buffer, size_t buffer_len) {
msg = sml_message_parse(buf);

if (sml_buf_has_errors(buf)) {
fprintf(stderr, "libsml: warning: could not read the whole file\n");
sml_error("warning: could not read the whole file");
break;
}

Expand Down
8 changes: 3 additions & 5 deletions sml/src/sml_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ sml_message_body *sml_message_body_parse(sml_buffer *buf) {
msg_body->data = sml_attention_response_parse(buf);
break;
default:
fprintf(stderr, "libsml: error: message type %04X not yet implemented\n", *(msg_body->tag));
sml_error("error: message type %04X not yet implemented", *(msg_body->tag));
break;
}

Expand Down Expand Up @@ -279,8 +279,7 @@ void sml_message_body_write(sml_message_body *message_body, sml_buffer *buf) {
sml_attention_response_write((sml_attention_response *)message_body->data, buf);
break;
default:
fprintf(stderr, "libsml: error: message type %04X not yet implemented\n",
*(message_body->tag));
sml_error("error: message type %04X not yet implemented", *(message_body->tag));
break;
}
}
Expand Down Expand Up @@ -334,8 +333,7 @@ void sml_message_body_free(sml_message_body *message_body) {
sml_attention_response_free((sml_attention_response *)message_body->data);
break;
default:
fprintf(stderr, "libsml: NYI: %s for message type %04X\n", __func__,
*(message_body->tag));
sml_error("NYI: %s for message type %04X", __func__, *(message_body->tag));
break;
}
sml_number_free(message_body->tag);
Expand Down
20 changes: 20 additions & 0 deletions sml/src/sml_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,29 @@
// along with libSML. If not, see <http://www.gnu.org/licenses/>.

#include <sml/sml_shared.h>
#include <stdarg.h>
#include <stdio.h>
#include <string.h>

void (*sml_error)(const char *format, ...);

void sml_error_default(const char *format, ...) {
va_list args;
va_start(args, format);

char *format2 = malloc(9 + strlen(format) + 1 + 1);
Copy link

Choose a reason for hiding this comment

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

you might check whether format is !=0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

seems somewhat esoteric, as format will usually be static anyway.
but might throw in a quick `format=format?format:"(null)";

Copy link

Choose a reason for hiding this comment

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

btw: why 9? the terminating zero is already at the end, or? So I'd expect
8 (= strlen("libsml: ") + ... + 1 (\n) + 1 (term. zero)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, i miscounted.

strcpy(format2, "libsml: ");
strcat(format2, format);
Copy link

Choose a reason for hiding this comment

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

strncpy/strncat? (some compilers/static code checker might sooner or later complain otherwise)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

seems needless as it's all static strings, i'd leave that to whoever adds that code-checker.

strcat(format2, "\n");

vfprintf(stderr, format2, args);

Copy link

Choose a reason for hiding this comment

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

va_end(args) missing?

free(format2);
}

// http://www.faqs.org/docs/Linux-HOWTO/Program-Library-HOWTO.html#INIT-AND-CLEANUP
void __attribute__((constructor)) sml_init() { sml_error = *sml_error_default; }

int sml_buf_get_next_length(sml_buffer *buf) {
int length = 0;

Expand Down
6 changes: 3 additions & 3 deletions sml/src/sml_transport.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ size_t sml_read(int fd, fd_set *set, unsigned char *buffer, size_t len) {
if (r < 0) {
if (errno == EINTR || errno == EAGAIN)
continue; // should be ignored
fprintf(stderr, "libsml: sml_read(): read error\n");
sml_error("sml_read(): read error");
return 0;
}
tr += r;
Expand All @@ -68,7 +68,7 @@ size_t sml_transport_read(int fd, unsigned char *buffer, size_t max_len) {

if (max_len < 8) {
// prevent buffer overflow
fprintf(stderr, "libsml: error: sml_transport_read(): passed buffer too small!\n");
sml_error("error: sml_transport_read(): passed buffer too small!");
return 0;
}

Expand Down Expand Up @@ -104,7 +104,7 @@ size_t sml_transport_read(int fd, unsigned char *buffer, size_t max_len) {
return len;
} else {
// don't read other escaped sequences yet
fprintf(stderr, "libsml: error: unrecognized sequence\n");
sml_error("error: unrecognized sequence");
return 0;
}
}
Expand Down
2 changes: 1 addition & 1 deletion sml/src/sml_tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ void sml_proc_par_value_write(sml_proc_par_value *value, sml_buffer *buf) {
sml_time_write(value->data.time, buf);
break;
default:
fprintf(stderr, "libsml: error: unknown tag in %s\n", __func__);
sml_error("error: unknown tag in %s", __func__);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sml/src/sml_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ double sml_value_to_double(sml_value *value) {
break;

default:
fprintf(stderr, "libsml: error: unknown type %d in %s\n", value->type, __func__);
sml_error("error: unknown type %d in %s", value->type, __func__);
return 0;
}
}
Expand Down
3 changes: 2 additions & 1 deletion test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ OBJS = \
src/sml_file_test.o \
src/sml_open_request_test.o \
src/sml_get_profile_pack_request_test.o \
src/sml_message_test.o
src/sml_message_test.o \
src/sml_error_test.o

test_run: libsml test
@./test
Expand Down
78 changes: 78 additions & 0 deletions test/src/sml_error_test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright 2020 volkszaehler.org
//
// This file is part of libSML.
//
// libSML is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// libSML is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with libSML. If not, see <http://www.gnu.org/licenses/>.

#include "../unity/unity_fixture.h"
#include "test_helper.h"
#include <sml/sml_shared.h>

#include <unistd.h>

TEST_GROUP(sml_error);

TEST_SETUP(sml_error) { }

TEST_TEAR_DOWN(sml_error) { }

TEST(sml_error, default) {
char *tmpfile="./test_sml_error.tmp"; // FIXME: should not be hardcoded
char *test_string="this is a test";
char *expected_output="libsml: this is a test\n";

FILE *capture = fopen(tmpfile, "w");
TEST_ASSERT_NOT_NULL(capture);
int stderr_backup=dup(2); // duplicate old stderr so it won't be closed
dup2(fileno(capture),2); // assign capture-file to stderr

sml_error(test_string);

fclose(capture);
dup2(stderr_backup,2); // restore stderr
close(stderr_backup); // discard backup

size_t len=strlen(expected_output)+1;
char *buf=malloc(len);

capture = fopen(tmpfile, "r");
size_t got=fread(buf,1,len-1,capture);
*(buf+got)=0; // add zero termination
//fprintf(stderr,"got: `%s`\n\n",buf);
fclose(capture);
unlink(tmpfile);

TEST_ASSERT_EQUAL(0, strcmp(expected_output,buf));

free(buf);
}

const char *msg=NULL;
void my_sml_error(const char *format, ... ){
msg=format;
}

TEST(sml_error, custom) {
char *test_string="this is a test";
sml_error = my_sml_error;
sml_error(test_string,1,2,3);
sml_error = sml_error_default;
TEST_ASSERT_NOT_NULL(msg);
TEST_ASSERT_EQUAL(0, strcmp(msg,test_string));
}

TEST_GROUP_RUNNER(sml_error) {
RUN_TEST_CASE(sml_error, default);
RUN_TEST_CASE(sml_error, custom);
}
1 change: 1 addition & 0 deletions test/test_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static void runAllTests() {
RUN_TEST_GROUP(sml_get_profile_pack_request);
RUN_TEST_GROUP(sml_message);
RUN_TEST_GROUP(sml_file);
RUN_TEST_GROUP(sml_error);
}

int main(int argc, char * argv[]) {
Expand Down