Re: ldso : dladdr support

[Szabolcs Nagy <nsz@port70.net>]

* musl <b.brezillon.musl@gmail.com> [2012-08-07 16:15:34 +0200]:
> On 07/08/2012 13:46, Szabolcs Nagy wrote:
> > * musl <b.brezillon.musl@gmail.com> [2012-08-07 11:04:19 +0200]:
> >> This patch adds support for dladdr function.
> >> It is based on my previous patch (gnu hash support).
> >>
> > i havent checked the content of your patch yet
> > just had a quick glance
> >
> > i think before you make substantial changes
> > it's better to have some discussion about
> > the design
> Could you tell me more about the design issues
> (I guess this has something to do with function pointers and multi hash algorithms design) ?


i didn't have anything specific in mind about the design
it was a general remark in case you plan to submit
larger contributions

about the function pointers:

using plain if/else might work, but if you use func pointers
then i'd make those part of the dso struct so you dont
need an additional lookup step, so

  dso->hash(s)

instead of

  hashalgo[dso->hashalg].hash(s)


in general keep the number of indirections and layers small
if possible


> >> +	uint32_t shift2 = hashtab[3];
> >> +	uint32_t h2 = h1 >> shift2;
> > i'm not sure if input validation makes sense in ldso
> > but shifts can be tricky (hashtab[3] must be in 0..31 here)
> By validation do you mean mask the shift value with 0x1F ?

i meant that if hashtab[3] comes from outside source
and ldso requires strict validation (i don't know if it does)
then you need to check the range of it somewhere
and return error for bad values