Skip to content

Instantly share code, notes, and snippets.

@rendello
Created January 2, 2025 02:54
Show Gist options
  • Save rendello/49c23aeba3734b9a3e6c443e9a2e493b to your computer and use it in GitHub Desktop.
Save rendello/49c23aeba3734b9a3e6c443e9a2e493b to your computer and use it in GitHub Desktop.
SQLite extension: Hex str to int conversion.
/*
** 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
Copy link

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.

  1. int base; is declared but not used. Was the intent to use this to control how strtoll() interprets the string? i.e. hex versus octal versus binary versus decimal.
    • This should cause a compiler warning. It's a good practice to treat warnings as seriously as errors.
  2. argc is untested, indeed unused.
    • Testing argc will allow you to gracefully exit if the wrong number of arguments are passed in.
    • If it's unused, you should be getting a warning. To tell the compiler you know it's unused, you can use the statement (void) argc; before you consume argv[].

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.

#define RADIX_BINARY  2
#define RADIX_OCTAL   8
#define RADIX_DECIMAL 10
#define RADIX_HEX     16

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.

@rendello
Copy link
Author

rendello commented Jan 23, 2025

@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 :)

@aodonnell-ca
Copy link

aodonnell-ca commented Jan 23, 2025 via email

@aodonnell-ca
Copy link

aodonnell-ca commented Jan 23, 2025 via email

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