-
-
Save rendello/49c23aeba3734b9a3e6c443e9a2e493b to your computer and use it in GitHub Desktop.
/* | |
** 2025-01-01 | |
** | |
** The author disclaims copyright to this source code. | |
*/ | |
#include "sqlite3ext.h" | |
SQLITE_EXTENSION_INIT1 | |
#include <stdlib.h> | |
#include <stdio.h> | |
/* | |
** Convert hex string to integer. Allow NULL to pass through, | |
** otherise the value must be a non-empty hex string with an optional | |
** '0x' or '0X' prefix. | |
*/ | |
static void hextoint( | |
sqlite3_context *context, | |
int argc, | |
sqlite3_value **argv | |
){ | |
sqlite_int64 result; | |
int base; | |
const unsigned char *start; | |
char *stop; | |
sqlite3_value *value = argv[0]; | |
int value_type = sqlite3_value_type(value); | |
switch (value_type) { | |
case SQLITE_NULL: | |
sqlite3_result_null(context); | |
return; | |
case SQLITE_TEXT: | |
start = sqlite3_value_text(value); | |
result = strtoll((const char *)start, &stop, 16); | |
if (*start == '\0' || *stop != '\0') | |
break; | |
sqlite3_result_int64(context, result); | |
return; | |
} | |
sqlite3_result_error(context, "expects hex string or NULL", -1); | |
} | |
#ifdef _WIN32 | |
__declspec(dllexport) | |
#endif | |
int sqlite3_hextoint_init( | |
sqlite3 *db, | |
char **pzErrMsg, | |
const sqlite3_api_routines *pApi | |
){ | |
int rc = SQLITE_OK; | |
SQLITE_EXTENSION_INIT2(pApi); | |
(void)pzErrMsg; /* Unused parameter */ | |
sqlite3_create_function(db, "hextoint", 1, SQLITE_UTF8|SQLITE_DETERMINISTIC, 0, hextoint, 0, 0); | |
return rc; | |
} |
@aodonnell-ca I figured, since I looked up your blog and saw you mention you did something similar there. I figured most C programmers wouldn't have an h1 heading entitled "pro tips", that's more of an LLM thing ;) For what it's worth, I think your original tone was good.
I think I'd disagree about the magic numbers in this case, I feel the fact that strtoll
takes a radix parameter, coupled with the fact that my short function is called hextoint
and has a documentation comment means that adding a #define
for a RADIX_HEX
constant would detract from the code, in my opinion.
Funnily enough I've also been working with code where strtok
seems like the first choice, but eventually I too decided not to use it. It's certainly an old-C-style function what with the in place modification.
Thanks again for taking time out of your lunch break to help review my code, I appreciate it :)
I actually wrote the original draft, markdown and all, but I wondered how my tone was. I handed it into ChatGPT and it noodled around a bit. Hence the language being finessed. I was on a coffee break, so was playing with Chat to help my writing style.
https://chatgpt.com/share/67928f6b-7d1c-8011-a13f-465d85aa57ab - you'll have to search to see the dialog switch to where I discuss this review, as it was in an already open session.
The Pre Chat review was as follows:
I'm following up on your request on LinkedIn for a review.
I don't do SQL, but I do C. There are a few questions I have.
int base;
is declared but not used. Was the intent to use this to control howstrtoll()
interprets the string? i.e. hex versus octal versus binary versus decimal.argc
is untested, indeed unused.(void) argc;
before you consumeargv[]
.About Warnings
Too many "benign" warnings in your code make it hard to see critical warnings.
Your unused
base
will be optimized out by the compiler, so it's not a big concern in itself.Your ignoring argc might be defensible, considering the consumer of argv can return an appropriate error state.
The more of these warnings you have, the less chance you have of spotting important warnings in your code.
Magic Numbers
Hard-coded "magic" numbers in code are regarded as not best practices. The code becomes more self documenting when named constants are used.
The 16 in stroll(), if you are reviewing the code and not famliar with stroll, you wouldn't necessarily know if that was a string length limit or a radix. Consider using the following defines for readability.
I see you also use a -1 as an error code. I don't know if this is a limitation or convention for SQLLite. If you are permitted other error codes, then again an enum or #define would help out. In this case it's not so great a foul.