[Crash-utility] [PATCH v3 4/4] Add assert check if an element is installed one more time

[Philipp Rudo]

Hi Tao,

On Tue, 14 Sep 2021 17:01:54 +0800
Tao Liu <ltao at redhat.com> wrote:

> symname_hash_install won't check if spn has been installed before. If
> it does, the second install will corrupt the hash table as well as
> spn->cnt counting. It should never happen in normal cases, but we need
> get notified if it happens.
> 
> Signed-off-by: Tao Liu <ltao at redhat.com>
> ---
>  symbols.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/symbols.c b/symbols.c
> index f8b4998..a453a6a 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -1167,6 +1167,7 @@ symname_hash_install(struct syment *table[], struct syment *spn)
>  		spn->name_hash_next = NULL;
>  	} else {
>  		while (sp) {
> +			assert(sp != spn);

the change makes totally sense. Although I'm not sure if the 'assert'
isn't too strict. In my opinion printing a warning + ignoring the
duplicate symbol would be better.

In my opinion an 'assert' should only be used, in situations a program
cannot be recovered from. But for the case when a symbol is added twice
we can 'recover' from by simply ignoring the second request.

Thanks
Philipp

>  	        	if (STREQ(sp->name, spn->name)) {
>  	                	sp->cnt++;
>  	                	spn->cnt++;