Skip to content

Comments

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

Open
r00t- wants to merge 1 commit intovolkszaehler:masterfrom
r00t-:sml_error_override
Open

WIP: add sml_error function pointer, allow user to intercept error messages#93
r00t- wants to merge 1 commit intovolkszaehler:masterfrom
r00t-:sml_error_override

Conversation

@r00t-
Copy link
Collaborator

@r00t- r00t- commented Dec 28, 2020

a basic attempt at fixing #17 .

replaces hardcoded fprintf() calls with an sml_error function pointer that can be overwritten by code using the library.
a default is provided that mimics the old behaviour (write to stderr with a "libsml:" prefix and appended newline.

includes a test, test also demonstrates usage.

note that this breaks backwards compatibility (code assuming sml_error exists won't build or run with older versions of the library),
so we should probably bump the version when merging this.

also we should consider if we maybe want a more sophisticated interface.

@r00t- r00t- force-pushed the sml_error_override branch 2 times, most recently from ab2acf3 to 9ad3a4d Compare December 28, 2020 03:07
@r00t- r00t- force-pushed the sml_error_override branch 2 times, most recently from 9e84db4 to b68e635 Compare December 28, 2020 03:15
@r00t- r00t- force-pushed the sml_error_override branch from b68e635 to 1028ef1 Compare December 28, 2020 03:28
Copy link

@mbehr1 mbehr1 left a comment

Choose a reason for hiding this comment

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

thx! see some comments/suggestions.

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?

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)";


char *format2 = malloc(9 + strlen(format) + 1 + 1);
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.

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.

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.

@r00t- r00t- changed the title add sml_error function pointer, allow user to intercept error messages WIP: add sml_error function pointer, allow user to intercept error messages Mar 10, 2021
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).

va_list args;
va_start(args, format);

char *format2 = malloc(9 + strlen(format) + 1 + 1);
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)";


char *format2 = malloc(9 + strlen(format) + 1 + 1);
strcpy(format2, "libsml: ");
strcat(format2, 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.

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

va_list args;
va_start(args, format);

char *format2 = malloc(9 + strlen(format) + 1 + 1);
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.

@jahir jahir mentioned this pull request Feb 6, 2023
@r00t-
Copy link
Collaborator Author

r00t- commented Feb 9, 2023

@jahir:
you beat me to mentioning this in #109 .
it's sad that this is stalled for 1.5 years, but it got very little feedback after i submitted it.
maybe you can add some?
i'm mostly wondering if the API is sensible, see #93 (comment) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants