Bug #627
closedcasts corrupt int values (on PPC)
Description
Debugging a module of my own, I encountered a problem that several config parameters of type int
were being corrupted in a reproducible way. The problem, as it seems, lies with passing numeric values in and out of arrays. When, in a SETDEFAULTS_FUNC, values are inserted from a config context in a plugin config, this eventually results in the following statement (config-glue.c, rev. 510 line 78, or rev. 1102 line 90, in function config_insert_values_internal
):
*((unsigned short *)(cv[i].destination)) = di->value;
The minimal test case is as follows:
#include <stdio.h> #include <stdlib.h> #include "array.h" /* From lighttpd */ void set(void *dest, data_unset *du) { *((unsigned short *)dest) = ((data_integer*)du)->value; } int main(int argc, char *argv[]) { int flags = 3000; int rcp; data_integer *di; di = data_integer_init(); di->value = flags; set(&rcp, (data_unset*) di); printf("%d = %x\n%d = %x\n", flags, flags, rcp, rcp); }
When compiled and run on my primary machine, which is a PowerPC, this prints:
3000 = bb8 196608000 = bb80000
I assume that this is due to PPC's being a big-endian architecture, but altogether, casting pointer types for variebyte data is somewhat careless. I have solved the problem locally by adding a new config_values_type_t
value T_CONFIG_INT
and a corresponding case in config_insert_values_internal
. I worked with the version of lighttpd from DarwinPorts, which is no longer current, and the modification is trivial, but if you are interested in my patches anyway, please tell where to send them.
Thank you.
-- boris.smilga
Files
Updated by moo almost 18 years ago
i don't think we need short+int at the same time. any way to analyze the code automatically?
Updated by agrostis almost 18 years ago
Up to you. It might at least be reasonable to change the type of value
in struct data_integer
(array.h rev. 1349, line 132) to unsigned short
as well.
I'm afraid I don't quite get your second remark.
Updated by moo almost 18 years ago
there're lots of lines to fix. either change CONFIG_SHORT to CONFIG_INTEGER and fix configure-glue.c, or change int to short. "any way to analyze the code automatically?" -> but do u think there's a way to analyze the code or anything so we can find out all the type mismatches? although i doubt it
Updated by agrostis almost 18 years ago
Replying to moo:
but do u think there's a way to analyze the code or anything so we can find out all the type mismatches? although i doubt it
There are, of course, lint and similar tools, such as SpLint, SMatch, and some commercial software. I only rarely program in C, and cannot really recommend anything.
Updated by stbuehler over 16 years ago
- Status changed from New to Fixed
- Resolution set to invalid
It looks like no module in upstream needs CONFIG_INTEGER, they alway use unsigned short for integer config options (if not, that would be a bug).
Perhaps it would be a good idea to convert all CONFIG_SHORT to CONFIG_INTEGER, but i see no bug here.
Also available in: Atom