Re: 330-gary-ltdl-vs-need-lib-prefix-unknown

[Ralf Wildenhues]

Hi Gary,

* Gary V. Vaughan wrote on Sat, Jun 23, 2007 at 06:17:56PM CEST:
> On Jun 23, 2007, at 5:27 AM, Ralf Wildenhues wrote:
> >* Gary V. Vaughan wrote on Thu, Jun 21, 2007 at 04:06:25AM CEST:
> 
> >I've retested now.  Things seem to work well for me, on GNU/Linux.
> >I do have a BeOS installation on my old, dusty PC, but it will be at
> >least a couple of weeks until I have enough time to actually try to
> >build and test Libtool HEAD on it.
> 
> I hope you don't mean you want the patch to bitrot for another couple of
> weeks before approving :-(

No, that's not what I meant to say.  But very likely I may not have
time until then anyway, being away for some days after Monday, and
then some more.  Maybe someone else can review them if I don't get
to it before.

> >>+#ifdef LT_DEBUG_LOADERS
> >>+  fprintf (stderr, "tryall_dlopen (%s, %s)\n", filename,
> >>+      vtable ? vtable->name : "(ALL)");
> >
> >Need to test filename == NULL and output "(null)" in that case.
> 
> Since it's just debug code, and fprintf usually handles that  
> automatically it didn't seem worth the trouble.

You can use

static const char *
non_null (const char *s)
{
  if (s) return s;
  else   return "(null)";
}

to avoid repetition.  But I think even debug code should be clean of
NULL pointer dereferences.

> >Inconsistent TAB vs. space indentation (yeah I'm nitpicky ;-).

> Maybe we should write a sed script that cleans this up?  I'll do it  
> by hand this time.

Oh, I don't think it's all that important.

> >I haven't been able to review the rest of the changes in
> >ltdl.c:tryall_dlopen and try_dlopen yet, sorry; the other changes in
> >your patch look good.  Judging from the number of different bits, I
> >wonder whether they bugs they fix are covered well enough in our
> >testsuite, though?
> 
> Absolutely our testsuite has less than full coverage.  Should we  
> start to record a TODO list of needed tests in CVS?

Feel free to.

Cheers,
Ralf