Project

General

Profile

Actions

Bug #627

closed

casts corrupt int values (on PPC)

Added by Anonymous over 18 years ago. Updated about 16 years ago.

Status:
Invalid
Priority:
Normal
Category:
core
Target version:
ASK QUESTIONS IN Forums:

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

src.ticket627.diff (1013 Bytes) src.ticket627.diff patches for ticket #627 in src/base.h, src/configfile-glue.h agrostis, 2006-11-19 22:35
Actions #1

Updated by moo over 18 years ago

simply attach file here, thanks.

Actions #2

Updated by agrostis almost 18 years ago

Sorry for the delay.

Actions #3

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?

Actions #4

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.

Actions #5

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

Actions #6

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.

Actions #7

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.

Actions #8

Updated by stbuehler about 16 years ago

  • Status changed from Fixed to Invalid
Actions

Also available in: Atom