Arnaud Lacombe <lacombar@xxxxxxxxx> wrote:
> On Tue, Sep 21, 2010 at 12:22 PM, Catalin Marinas
> <catalin.marinas@xxxxxxx> wrote:
>> Arnaud Lacombe <lacombar@xxxxxxxxx> wrote:
>>> On Tue, Sep 21, 2010 at 8:32 AM, Catalin Marinas
>>> <catalin.marinas@xxxxxxx> wrote:
>>>> From: Catalin Marinas <catalin.marinas@xxxxxxx>
>>>>
>>>> Commit 246cf9c26b introduced the tracking of direct dependency to
>>>> provide additional warning when they are not met. With some complex
>>>> dependencies, the expr_copy() function called on such expressions may
>>>> cause stack exhaustion. The patch removes the superfluous expr_copy()
>>>> call since expr_transform handles symbol duplication already.
[...]
>> Does the patch below make it any better (together with the previous
>> one)?
>>
> just this one is enough. I added back the memory poisoning in
> expr_free() and did not get any message of invalid expression.
Thanks for testing. I would add the other patch as well, maybe as a
separate one since it's a clean-up.
>> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
>> index 7298806..e707aa2 100644
>> --- a/scripts/kconfig/menu.c
>> +++ b/scripts/kconfig/menu.c
>> @@ -107,7 +107,9 @@ static struct expr *menu_check_dep(struct expr *e)
>> Âvoid menu_add_dep(struct expr *dep)
>> Â{
>> Â Â Â Âcurrent_entry->dep = expr_alloc_and(current_entry->dep, menu_check_dep(dep));
>> - Â Â Â current_entry->dir_dep = current_entry->dep;
>> + Â Â Â if (current_entry->dir_dep)
>> + Â Â Â Â Â Â Â expr_free(current_entry->dir_dep);
> the test is not needed, expr_free() will return immediately if (e == NULL).
>
>> + Â Â Â current_entry->dir_dep = expr_copy(current_entry->dep);
>
> FYI, just this line makes the crash disappear, still I understand the
> expr_free(). However, wouldn't there be a better place to initialize
> `dir_dep' and avoid the need to use expr_free() ?
dir_dep would need to be initialised somewhere, unless there is a memset
on the whole structure (I'm not familiar enough with kbuild). But I
think we still need the expr_free() in case dir_dep gets overwritten
(e.g. multiple 'depends on' lines where the full expression is re-built
on the previous line via expr_alloc_and()).
I'll post some patches tomorrow to be reviewed (have to go now).
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
