From 3a082b7f2a92bad9630fd931fff58e33057e537c Mon Sep 17 00:00:00 2001 From: BVK Chaitanya Date: Thu, 22 Jul 2010 04:49:05 +0530 Subject: [PATCH 1/2] memory management for block parameters --- include/grub/script_sh.h | 13 +++++++++- script/argv.c | 3 +++ script/execute.c | 2 +- script/main.c | 2 +- script/parser.y | 53 ++++++++++++++++++++++++++++++++++++++-- script/script.c | 10 ++++++++ 6 files changed, 78 insertions(+), 5 deletions(-) diff --git a/include/grub/script_sh.h b/include/grub/script_sh.h index 43791d040..7618633ad 100644 --- a/include/grub/script_sh.h +++ b/include/grub/script_sh.h @@ -42,6 +42,10 @@ struct grub_script unsigned refcnt; struct grub_script_mem *mem; struct grub_script_cmd *cmd; + + /* Other grub_script's from block arguments. */ + struct grub_script *siblings; + struct grub_script *children; }; typedef enum @@ -222,6 +226,9 @@ struct grub_parser_param /* The memory that was used while parsing and scanning. */ struct grub_script_mem *memused; + /* The block argument scripts. */ + struct grub_script *scripts; + /* The result of the parser. */ struct grub_script_cmd *parsed; @@ -365,13 +372,17 @@ grub_normal_parse_line (char *line, grub_reader_getline_t getline); static inline struct grub_script * grub_script_get (struct grub_script *script) { - script->refcnt++; + if (script) + script->refcnt++; return script; } static inline void grub_script_put (struct grub_script *script) { + if (! script) + return; + if (script->refcnt == 0) grub_script_free (script); else diff --git a/script/argv.c b/script/argv.c index 42d573a0a..a7acbc23e 100644 --- a/script/argv.c +++ b/script/argv.c @@ -52,9 +52,12 @@ grub_script_argv_free (struct grub_script_argv *argv) grub_free (argv->args); } + if (argv->script) + grub_script_put (argv->script); argv->argc = 0; argv->args = 0; + argv->script = 0; } /* Prepare for next argc. */ diff --git a/script/execute.c b/script/execute.c index 932be6635..b9538c29b 100644 --- a/script/execute.c +++ b/script/execute.c @@ -201,7 +201,7 @@ grub_script_arglist_to_argv (struct grub_script_arglist *arglist, grub_script_argv_append (&result, arg->str) || grub_script_argv_append (&result, "}")) goto fail; - result.script = arg->script; + result.script = grub_script_get (arg->script); break; case GRUB_SCRIPT_ARG_TYPE_TEXT: diff --git a/script/main.c b/script/main.c index 752a8cd8a..19ce88c43 100644 --- a/script/main.c +++ b/script/main.c @@ -34,7 +34,7 @@ grub_normal_parse_line (char *line, grub_reader_getline_t getline) grub_script_execute (parsed_script); /* The parsed script was executed, throw it away. */ - grub_script_free (parsed_script); + grub_script_put (parsed_script); } return grub_errno; diff --git a/script/parser.y b/script/parser.y index 76dc3fbca..4f57ea4ff 100644 --- a/script/parser.y +++ b/script/parser.y @@ -38,6 +38,7 @@ struct { unsigned offset; struct grub_script_mem *memory; + struct grub_script *scripts; }; } @@ -152,16 +153,45 @@ argument : "case" { $$ = grub_script_add_arglist (state, 0, $1); } | word { $$ = $1; } ; +/* + Block parameter is passed to commands in two forms: as unparsed + string and as pre-parsed grub_script object. Passing as grub_script + object makes memory management difficult, because: + + (1) Command may want to keep a reference to grub_script objects for + later use, so script framework may not free the grub_script + object after command completes. + + (2) Command may get called multiple times with same grub_script + object under loops, so we should not let command implementation + to free the grub_script object. + + To solve above problems, we rely on reference counting for + grub_script objects. Commands that want to keep the grub_script + object must take a reference to it. + + Other complexity comes with arbitrary nesting of grub_script + objects: a grub_script object may have commands with several block + parameters, and each block parameter may further contain multiple + block parameters nested. We use temporary variable, state->scripts + to collect nested child scripts (that are linked by siblings and + children members), and will build grub_scripts tree from bottom. + */ block: "{" { grub_script_lexer_ref (state->lexerstate); $$ = grub_script_lexer_record_start (state); $$ = grub_script_mem_record (state); + + /* save currently known scripts. */ + $$ = state->scripts; + state->scripts = 0; } commands1 delimiters0 "}" { char *p; struct grub_script_mem *memory; + struct grub_script *s = $2; memory = grub_script_mem_record_stop (state, $2); if ((p = grub_script_lexer_record_stop (state, $2))) @@ -171,6 +201,19 @@ block: "{" if (! $$ || ! ($$->script = grub_script_create ($3, memory))) grub_script_mem_free (memory); + else { + /* attach nested scripts to $$->script as children */ + $$->script->children = state->scripts; + + /* restore old scripts; append $$->script to siblings. */ + state->scripts = $2 ?: $$->script; + if (s) { + while (s->siblings) + s = s->siblings; + s->siblings = $$->script; + } + } + grub_script_lexer_deref (state->lexerstate); } ; @@ -243,10 +286,13 @@ commands1: newlines0 command } ; -function: "function" "name" +function: "function" "name" { grub_script_lexer_ref (state->lexerstate); state->func_mem = grub_script_mem_record (state); + + $$ = state->scripts; + state->scripts = 0; } delimiters0 "{" commands1 delimiters1 "}" { @@ -256,9 +302,12 @@ function: "function" "name" script = grub_script_create ($6, state->func_mem); if (! script) grub_script_mem_free (state->func_mem); - else + else { + script->children = state->scripts; grub_script_function_create ($2, script); + } + state->scripts = $3; grub_script_lexer_deref (state->lexerstate); } ; diff --git a/script/script.c b/script/script.c index 7cf2f6bae..6509b5f5d 100644 --- a/script/script.c +++ b/script/script.c @@ -94,12 +94,19 @@ grub_script_mem_record_stop (struct grub_parser_param *state, void grub_script_free (struct grub_script *script) { + struct grub_script *s; + if (!script) return; if (script->mem) grub_script_mem_free (script->mem); + s = script->children; + while (s) { + grub_script_put (s); + s = s->siblings; + } grub_free (script); } @@ -346,6 +353,8 @@ grub_script_create (struct grub_script_cmd *cmd, struct grub_script_mem *mem) parsed->mem = mem; parsed->cmd = cmd; parsed->refcnt = 0; + parsed->siblings = 0; + parsed->children = 0; return parsed; } @@ -394,6 +403,7 @@ grub_script_parse (char *script, grub_reader_getline_t getline) parsed->mem = grub_script_mem_record_stop (parsestate, membackup); parsed->cmd = parsestate->parsed; + parsed->children = parsestate->scripts; grub_script_lexer_fini (lexstate); grub_free (parsestate); From 1d0546767810ccce6c2090b675f6b5006f2b307a Mon Sep 17 00:00:00 2001 From: BVK Chaitanya Date: Thu, 22 Jul 2010 05:05:49 +0530 Subject: [PATCH 2/2] simplify --- commands/extcmd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/commands/extcmd.c b/commands/extcmd.c index cc0362e14..ecf2d4133 100644 --- a/commands/extcmd.c +++ b/commands/extcmd.c @@ -40,9 +40,7 @@ grub_extcmd_dispatcher (struct grub_command *cmd, int argc, char **args, context.extcmd = ext; context.script = script; - /* Dynamic commands should not perform option parsing before - corresponding module gets loaded. */ - if (cmd->flags & GRUB_COMMAND_FLAG_DYNCMD) + if (! ext->options) { ret = (ext->func) (&context, argc, args); return ret;