====== Differences ====== This shows you the differences between two versions of the page.
| Next revision | Previous revision | ||
|
soc:2009:lynusvaz:journal:week10 [2009/07/28 11:27] lynusvaz created |
soc:2009:lynusvaz:journal:week10 [2009/08/02 10:50] (current) lynusvaz |
||
|---|---|---|---|
| Line 4: | Line 4: | ||
| * Modify strtoul to handle a +/- sign (there are cases when I need to 'recognize' negative numbers) | * Modify strtoul to handle a +/- sign (there are cases when I need to 'recognize' negative numbers) | ||
| However, it's turning out to be a rather difficult task. The err_val and tok should have been easy to unify, but there seems to be a memory leak problem, when an error is encountered after a string token is found (and allocated). The string management is another problem. The parse_arith() function takes a struct string as input. The input() function goes along the same struct string to get the next token. But, the parsing functions dynamically allocate space for the result. I find this tree-like method of operation simpler to handle. But this can easily cause memory leak problems on parse errors, which is something I'm still trying to fix. | However, it's turning out to be a rather difficult task. The err_val and tok should have been easy to unify, but there seems to be a memory leak problem, when an error is encountered after a string token is found (and allocated). The string management is another problem. The parse_arith() function takes a struct string as input. The input() function goes along the same struct string to get the next token. But, the parsing functions dynamically allocate space for the result. I find this tree-like method of operation simpler to handle. But this can easily cause memory leak problems on parse errors, which is something I'm still trying to fix. | ||
| + | |||
| + | July 29-31: Unified the err_val and tok variables. The tok variable now takes negative values on errors, and the 'forgotten' string token is freed before setting it. It's back to valgrind and gcov for testing. An interesting issue I didn't notice is using an int to store the difference between pointers. Apparently this will not work in a 64-bit environment, so I had to use longs at all these places. Size check: Before (official gPXE source): | ||
| + | 321 16 4 341 155 bin/shell.o | ||
| + | 548 88 0 636 27c bin/bios_console.o | ||
| + | 629 8 0 637 27d bin/exec.o | ||
| + | 2365 95 2 2462 99e bin/comboot_call.o | ||
| + | 81784 11300 139774 232858 38d9a (TOTALS) | ||
| + | After (quoting and arithmetic parser): | ||
| + | 11 24 0 35 23 bin/serial_console.o | ||
| + | 335 16 4 355 163 bin/shell.o | ||
| + | 577 8 4 589 24d bin/exec.o | ||
| + | 1322 0 0 1322 52a bin/parse.o | ||
| + | 2232 0 20 2252 8cc bin/arith.o | ||
| + | 2377 95 2 2474 9aa bin/comboot_call.o | ||
| + | 84571 11236 139798 235605 39855 (TOTALS) | ||
| + | And the bin/rtl8139.rom has grown from 55808 to 57344B. Latest commit in the [[http://git.etherboot.org/?p=people/lynusvaz/gpxe.git;a=shortlog;h=refs/heads/scripting_test|scripting_test]] branch: [[http://git.etherboot.org/?p=people/lynusvaz/gpxe.git;a=commit;h=d6f7017ac5827009d21648b079538fb5eeaed002|Arithmetic parser]] | ||
| + | |||
| + | August 1: Weekly meeting today. Today's topic was getting the patches merged into gPXE. Had a discussion with mcb30 on the generic stack implementation. The stack and struct string idea was started to let the parser work without having to deal with issues like memory allocation, and also to help other modules that will use stacks. The current implementation is just a wrapper around an array to push and pop elements. The disadvantage is that since the array is fixed, the stack has a limited size. mcb30 recommended that I use the list_head structure and macros to implement an unbounded stack, that can accomodate as many elements as required. The basic skeleton is: | ||
| + | struct stack { | ||
| + | struct list_head list; | ||
| + | }; | ||
| + | | ||
| + | struct stack_element { | ||
| + | struct list_head list; | ||
| + | char data[0]; | ||
| + | }; | ||
| + | | ||
| + | #define STACK( _type ) \ | ||
| + | union { \ | ||
| + | struct stack stack; \ | ||
| + | _type type[0]; \ | ||
| + | } | ||
| + | | ||
| + | #define STACK_PUSH( _stack, _data_len ) \ | ||
| + | ( ( typeof ( &(_stack)->type[0] ) ) \ | ||
| + | stack_push ( (_stack)->stack, _data_len ) ) | ||
| + | | ||
| + | #define STACK_POP( _item ) \ | ||
| + | stack_pop ( _item ); | ||
| + | | ||
| + | void * stack_push ( struct stack stack, size_t data_len ) { | ||
| + | struct stack_element *element; | ||
| + | element = malloc ( sizeof ( *element ) + data_len ); | ||
| + | | ||
| + | if ( ! element ) | ||
| + | return NULL; | ||
| + | | ||
| + | list_add_tail ( &element->list, &stack->list ); | ||
| + | return &element->data; | ||
| + | } | ||
| + | | ||
| + | void stack_pop ( void *data ) { | ||
| + | struct stack_element *element = | ||
| + | container_of ( data, struct stack_element, data ); | ||
| + | list_del ( &element->list ); | ||
| + | free ( element ); | ||
| + | } | ||
| + | |||
| + | August 2: Worked on the stack today. A few modifications to yesterday's ideas: | ||
| + | * Added a stack_for_each macro to help iteration over the stack. | ||
| + | * I had some problems with passing the stack to a function. The type information is held in the union, but actually passing the union to a function needs to have the data type clearly defined. So, I passed only a pointer to a struct stack, which removed the type information. | ||
| + | * Hence, I made the STACK_PUSH macro take the type of the new element, and call the push_stack function with the sizeof(type) argument. | ||
| + | |||
| + | #define PUSH_STACK( _stack, _type ) ( _type * ) stack_push ( _stack, sizeof ( _type ) ) | ||
| + | |||
| + | This will retain the type information without the union, but it requires the user to know the type of element being pushed, and this may lead to mistakes. This stack implementation seems to add around 135B to the uncompressed code. | ||
| + | |||
