[Crash-utility] [PATCH v4 2/4] Get the absolute value of SYMNAME_HASH_INDEX

[Philipp Rudo]

Hi Tao,
Hi Lianbo,

sorry for the late response.

On Wed, 29 Sep 2021 15:46:19 +0800
lijiang <lijiang at redhat.com> wrote:

> > > > Currently, the macro is used twice in the symbols.c. This change seems
> > > > not complicated. Any thoughts?  
> > >
> > > do I understand your suggestion correct, you propose to replace the
> > >
> > >         #define SYMNAME_HASH_INDEX(name) ...
> > >
> > > in defs.h by something like
> > >
> > >         static unsigned long long SYMNAME_HASH_INDEX(const unsigned char * const name) {
> > >                 return (name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2]) % SYMNAME_HASH);
> > >         }
> > >
> > > in symbols.c? If so, I think that should be fine.
> > >  
> Yes, you are right, Philipp.
> 
> >
> > Please correct me if I'm wrong. I don't think the function can work.
> > Let's say name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2]) ==
> > -1, then we will have:
> >
> > static unsigned long long SYMNAME_HASH_INDEX(const unsigned char * const name) {
> >         return (-1) % 512;
> > }
> >
> > The returned value is a very large number, and will overflow the array.

@Tao: I don't think that should be possible here. In the function I
have defined name as _unsigned_ char*. So the calculation should never
become negative. But to be honest I haven't tested it.

> No, this is a modulo operation, and its result will never exceed '512' anyway.
> (unsigned long long)(-1) % 512 = 511

@Lianbo: I don't think that's true. When the modulo is used with an
negative dividend the result becomes negative. When this then gets
casted to an unsigend integer type the result will become very large.
That's also what a quick test showed me

printf("%lli\n", ((-1LL) % 512));			--> -1
printf("%llu\n", (unsigned long long) ((-1LL) % 512));  --> 18446744073709551615

Thanks
Philipp

> 
> Thanks.
> Lianbo
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility
>