[Crash-utility] [PATCH v3 4/4] Add assert check if an element is installed one more time
Tao Liu
ltao at redhat.com
Sat Sep 18 06:36:50 UTC 2021
On Fri, Sep 17, 2021 at 5:09 PM Philipp Rudo <prudo at redhat.com> wrote:
>
> 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.
>
OK. I will replace it with a function which can check if spn exists in
the hashtable.
Thanks,
Tao Liu
> Thanks
> Philipp
>
> > if (STREQ(sp->name, spn->name)) {
> > sp->cnt++;
> > spn->cnt++;
>
More information about the Crash-utility
mailing list