hiredis SET runs into segmentation fault

Question

I'm trying to SET a struct into Redis with hiredis:

struct StatLite
{
    uid_t uid;
    gid_t gid;
    mode_t mode;
}

bool RedisPermissionHandler::Set(std::string path, StatLite stat)
{
    redisReply *reply = (redisReply*)redisCommand(this->redis,
        "SET %b %b",
        path.c_str(), (size_t)path.length(),
        stat, (size_t)sizeof(stat));
    freeReplyObject(reply);
    return true;
}

However this runs into a segmentation fault somewhere inside hiredis.

this->redis, path, and stat have proper values. GET commands work and deliver a NIL reply type (since Redis is empty).

What am I doing wrong?

@tadman VSCode thinks `redisCommand` returns `void*`, so just to make it shut up. — Cobra_Fast, Nov 04 '19 at 02:06
I just looked at the docs and wow, yeah, it is `void*`. You sure you don't want to use a C++ connector here? If you mis-cast this anything could happen. — tadman, Nov 04 '19 at 02:07
@tadman hiredis is the only one easily available through linux package repos. And from the docs it seemed straightforward enough. I'm open to recommendations though. — Cobra_Fast, Nov 04 '19 at 02:09
C adapters often have to deal with a lot of C limitations. There's a whole [heap of C++ clients](https://redis.io/clients#c--) you could use instead. Surely one of those is better. You can build that as a library and then fold that into your final executable. — tadman, Nov 04 '19 at 02:10
@tadman Virtually all of these either wrap hiredis or require boost. So what do? — Cobra_Fast, Nov 04 '19 at 02:17
Just an idea. You may need to step through your code with a debugger to find out where it explodes and then work back to figure out what you did wrong. Are you compiling with aggressive warnings (`-Wall`) on? Are you sure `this->redis` is populated correctly? — tadman, Nov 04 '19 at 02:19
@tadman As stated in OP, `GET` commands seem to work fine, so `this->redis` is probably okay. I do compile with `-Wall` and the only warning is about something completely unrelated. — Cobra_Fast, Nov 04 '19 at 02:23
Hmm. Can you really slam in a `stat` as if it's a string without any sort of conversion? You're not even sending it a pointer, just some kind of raw `struct`. — tadman, Nov 04 '19 at 02:26
@tadman The redis docs say "Values can be strings (including binary data) of every kind". So I assumed it was fine. I can try base64 encoding or something. — Cobra_Fast, Nov 04 '19 at 02:32
You may need a pointer here instead. You're passing the `struct`. Try `&stat`. — tadman, Nov 04 '19 at 02:33
@tadman Oh boy, yeah, giving it `&stat` seems to work... Want to put that into an answer for reputation? — Cobra_Fast, Nov 04 '19 at 02:33

score 4 · Accepted Answer · answered Nov 04 '19 at 02:36

The trouble here is that you're specifying a raw structure instead of a pointer to the structure:

bool RedisPermissionHandler::Set(std::string path, StatLite stat)
{
    redisReply *reply = (redisReply*)redisCommand(this->redis,
        "SET %b %b",
        path.c_str(), (size_t)path.length(),
        &stat, (size_t)sizeof(stat) // Pointer to stat!
    );

    freeReplyObject(reply);
    return true;
}

It's probable that the driver was looking for a void* buffer of a particular size and treated stat as a void*, causing a segfault when that pointer got de-referenced.

hiredis SET runs into segmentation fault

1 Answers1