[haiku-commits] Re: BRANCH waddlesplash-github.libroot-sha256 [e6a3dfe38b01] src/system/libroot/posix/crypt src/kits/shared headers/private/libroot src headers/private/shared

  • From: Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 2 Oct 2017 19:46:59 +0200

On Mon, Oct 02, 2017 at 05:09:57PM +0200, waddlesplash wrote:

On Mon, Oct 2, 2017 at 1:56 PM, Rene Gollent <anevilyak@xxxxxxxxx> wrote:
This explanation makes no real sense in any way. libshared is a static
library, not a DSO. As such, when linking it to libroot, only the
pieces libroot actually references (e.g. SHA256.o) are included in the
final libroot that's produced as a result, which is functionally no
different from what you're proposing to do (and hardly a "terrible
idea").

I hope so, yes. PulkoMandy thought that we shouldn't be linking
libshared to libroot at all, though he didn't give a definite reason
as such.

The reason is:

- The goal of libshared is to put stuff with unfinished APIs, which may
  change or be removed if we change our mind,
- If we link such stuff inside libroot, applications linking libroot
  will be able to use them without lnking to libshared, so the API is
  exposed to unsuspecting application developers (this does not mean the
  whole libshared is included, just the part libroot uses as Rene
  wrote),
- So, there is no point in putting the stuff in libshared if is already
  is in libroot anyway.

Conclusion: let's be clear about what we mean, and if we need this code
in libroot, let's have it there.


Furthermore, BControlLook lives in libbe, not libshared or
libroot, so I have no idea how you think the issues are in any way
conflated. If it's a result of ABI changes, then the implication is
that packages haven't properly been rebuilt against a newer Haiku with
the changes.

The problem is that two of the crashing applications (Deskbar and
DriveSetup) link to nothing outside the tree AFAICT, and the one thing
that is a port (Python) does not link to libbe. Furthermore, the crash
function in DriveSetup [1] is a simple return-member-variable, 

Looking at the debug report, it is called on a NULL pointer (you can see
the register being dereferenced is 0), which perfectly explains the
crash even for such a simple function. It looks like the BMenu was not
created, or deleted while its thread was running.

and the
crash function in Deskbar is a static initializer. And there really
weren't any other changes which should have affected something like
this. So a wild hunch, yes, but ...

I would be uncomfortable if this fixed the problems without giving an
explanation. Then the bug may pop up again under other circumstances and
we would still not know what the problem is.

We need to get down to the root cause and fix it for real, not shuffle
the code until it seems to work again.

Moving SHA256 into libroot is still a good move in terms of
architecture, but is unlikely to help with the initial problem.

-- 
Adrien.

Other related posts: