uninitialised value inside redisvFormatCommand()

[mezzatto]

Looks like redisFormatCommand() supports just 2 binary strings (%b). If you use de third binary string something goes wrong inside redisFormatCommand().

Inside test.c there is this code:

test("Format command with %%b string interpolation: ");
len = redisFormatCommand(&cmd,"SET %b %b","foo",3,"b\0r",3);

I have changed it to this (added 1 more binary string):

test("Format command with %%b string interpolation: ");
len = redisFormatCommand(&cmd,"SET %b %b %b","foo",3,"b\0r",3,"\t\n\r\t",4);

The above code causes an uninitialised value here:

        case 'b':
            arg = va_arg(ap,char*);
            size = va_arg(ap,size_t);
            if (size > 0)
                current = sdscatlen(current,arg,size);
            interpolated = 1;
            break;

For some reason, "size" (in "size = va_arg(ap,size_t);") becomes uninitialised.

I have discovered this problem by running de modified test.c inside valgrind (changed Makefile to -O0 also), which gererated the following error:

==30230== Conditional jump or move depends on uninitialised value(s)
==30230== at 0x50B4B17: redisvFormatCommand (hiredis.c:633)
==30230== by 0x50B50C8: redisFormatCommand (hiredis.c:744)
==30230== by 0x40147A: test_format_commands (test.c:58)
==30230== by 0x40316E: main (test.c:484)

I ran valgrind like this:

valgrind --leak-check=full --leak-resolution=high ./hiredis-test

This problem is not allowing me to use HMSET to set several fileds at once. I had to call HSET many times or use %s instead of %b (which is not efficient, since I already know the size of the string and I dont want another strlen()).

[pietern]

Thanks. This is hard to fix in a portable way, since it depends on how the compiler handles variadic functions. See this SO post: http://stackoverflow.com/questions/727663/why-does-this-variadic-function-fail-on-4th-parameter-on-windows-x64

redisFormatCommand expects the length arguments to have type size_t (on 64-bit systems to be 8 bytes). When an int is passed, there are uninitialized bits and the extracted value will be way off. You can fix this by manually casting the length arguments to size_t. Changing the type of the length argument that redisFormatCommand accepts will break other code and introduce the same issue the other way around (explicitly cast size_t to int, for instance).

On another note: check out redisFormatCommandArgv for commands with a variable number / large number of arguments. This doesn't require you to fix up the format string every time you want to add arguments.

[mezzatto]

Casting to size_t worked for me. Thanks.