[haiku-commits] Re: haiku: hrev52665 - src/system/runtime_loader

  • From: looncraz <looncraz@xxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 20 Dec 2018 09:44:26 -0600

On 12/20/2018 03:12, Jérôme Duval wrote:

Am Mi., 19. Dez. 2018 00:02 hat waddlesplash <waddlesplash@xxxxxxxxx <mailto:waddlesplash@xxxxxxxxx>> geschrieben:

    hrev52665 adds 1 changeset to branch 'master'
    old head: 296120e005ab57970d8eb9979e9e42c974c4978c
    new head: 32560010419a8fb79fdaf335ab066d2133fec700


    32560010419a: runtime_loader: Reinit the heap lock after fork.

                                  [ Augustin Cavalier
    <waddlesplash@xxxxxxxxx <mailto:waddlesplash@xxxxxxxxx>> ]

    diff --git a/src/system/runtime_loader/export.cpp
    index a480210cbd..81146ff2eb 100644
    --- a/src/system/runtime_loader/export.cpp
    +++ b/src/system/runtime_loader/export.cpp
    @@ -43,6 +43,18 @@ export_unload_library(void* handle)

    +       status_t returnstatus = B_OK;
    +       if (status_t status = elf_reinit_after_fork())
    +               returnstatus = status;
    +       if (status_t status = heap_reinit_after_fork())
    +               returnstatus = status;
    +       return returnstatus;

status should be checked against B_OK. Why not just set returnstatus?


PS: I would have preferred to review on Gerrit...

B_OK evaluates to false while B_ERROR and so on evaluate to true.

In the event the first conditional returns an error 'returnstatus' will be set with the error - this could be replaced simply by

    status_t returnstatus = elf_reinit_after_fork();

The second conditional, however, is necessary to prevent B_OK from clobbering a bad 'returnstatus'

Then it's a matter of whether it's desirable to run heap_reinit_after_fork() if ELF reinit failed.  If not, then the following code is better, IMHO:

    if (returnstatus == B_OK)
        returnstatus = heap_init_after_fork();

--The loon

Other related posts: