Hi! I've recently used some tool to check redis code (unstable branch, newest version from github) and find that there are some unused value assignments in the code base. When a developer assigns some value to a variable, the expectation is to use the value somewhere. So the unused assignments could potentially indicate bugs:
| File | Variable | Line |
|---|---|---|
| redis/src/defrag.c | size | 61 |
| redis/deps/jemalloc/src/ctl.c | lg_sample | 2614 |
| redis/src/sds.c | f | 638 |
| redis/src/sentinel.c | sections | 3475 |
| redis/src/t_stream.c | p | 3120 |
| redis/src/t_stream.c | p | 3123 |
I will appreciate it so much if you could take a bit time to confirm.
Even if they are not bugs, they may invoke compiler warnings and may still deserve to be dealt with. I wonder if I can help to remove them so that they can keep quiet when compiled.
Thanks!
Comment From: madolson
The stream ones seemed misaligned, but honestly these don't seem terribly important to fix. I presume a modern compiler would optimize it away.
Comment From: oranagra
the one in defrag.c can't be optimized by the compiler, the method that is called is not declared a pure function, and it is implemented in a different module, and even calls a function from another library (which might even be a shared library). so the compiler doesn't know what it does, and if the call performs any actions rather than the value it returns.
so i'd like to fix this one.
the ones in t_stream.c seems like a false report (or maybe i need to get my eyes checked). the other ones are nonsense, i don't mind cleaning them up, but they also don't bother me.
@FloridSleeves do you wanna make a PR to fix defrag.c, or shall I do that?
Comment From: oranagra
ohh, i do need to get my eyes checked. i see the complaint in t_stream.c
this unused variable is there on purpose, it's a pattern.
i keep the pointer to the current record, then i "validate" it by stepping to the next record, and only then after i know it's valid, i sometimes use p to read it's content.
so all the calls to lpValidateNext, first keep p before stepping.
Comment From: FloridSleeves
Thank you so much for your confirmation and patch!