C++ proposition

[apianti]

2 hours ago, Slice said:

I don't know yet how to call default destructors before calling boot.efi. Something like CloverGUI.Finish(); after OnExitBootServices.

 

You can't modify the memory map during this event, or at any point during the hook or call to exit. So as long as no destructor ever free'd memory you'd be fine, otherwise you'd have to call them all individually without freeing anything. Any memory that is allocated as boot type is gone anyway.

[Slice]

48 minutes ago, Sherlocks said:

i'm finished latest clover build.

but i'm always getting black screen after enter mac disk.

i'm using buildme with xcode8 option and xcode 11 beta.

is it right? in last official release(https://github.com/CloverHackyColor/CloverBootloader/releases), there is no problem.

 

This is known problem

 

[Jief_Machak]

2 hours ago, Slice said:

Hang between

DBG ("stringlength = %d\n", device_inject_stringlength);

and

MsgLog ("CurrentMode: Width=%d Height=%d\n", UGAWidth, UGAHeight);

There are many FreePool(ptr) which may hang if ptr is not valid pointer.

It was working before C++. Where is the influence?

I've update and it and it runs.

Does that happens only with xcode 11 beta ?

I can setup a virtual machine with the same os x and the same xcode, if you'd like me to test.

I've re-enabled real-time debugging so it'll be quick for me to see where it hangs (no need to add DBG("bla") etc.).

[Sherlocks]

1 hour ago, Jief_Machak said:

Could you try a build with GCC ?

both XCODE8 and GCC, black screen still happen

[Slice]

2 minutes ago, Sherlocks said:

both XCODE8 and GCC, black screen still happen

I reproduced this issue.

[Slice]

I compiled by GCC53.

The issue happen with real hardware, not present with QEMU, probably because QEMU settings doesn't contain Properties.

[Slice]

There is another issue I encounter

https://www.insanelymac.com/forum/topic/341986-c-proposition/?do=findComment&comment=2709219

I traced both issues and narrow the place of hang. The place is floating that usually means memory problem.

In the both case there is 

Status = gBS->AllocatePages (

                                 AllocateMaxAddress,

                                 EfiACPIReclaimMemory,

                                 EFI_SIZE_TO_PAGES (device_inject_stringlength + 1),

                                 &BufferPtr

                                 );

I have to propose that something wrong with the call. It works with Clover 5104 release so something changed with C++. May be EFI_SIZE_TO_PAGES?

[Pene]

1 hour ago, Slice said:

I have to propose that something wrong with the call. It works with Clover 5104 release so something changed with C++. May be EFI_SIZE_TO_PAGES?

 

7 hours ago, Jief_Machak said:

I can't, so please explain your configuration, so I can try.

 

On 2/25/2020 at 7:05 AM, MakAsus said:

I go to the Clover GitHub repository, download the source code of the last commit, then run the ./buildme script from the CloverBootloader folder, after completing the clover building, install it and get a black screen without any text after selecting the drive and pressing the enter key.

 

10 hours ago, Sherlocks said:

i'm finished latest clover build.

but i'm always getting black screen after enter mac disk.

 

There was a mistake in memory allocation of devtree nodes.

sizeof(__typeof_am__(*Child)) will give 8 while sizeof(DEV_PROPERTY) is much more than that. This must result in all kind of weird errors.

Please try again after my last commit.

[Slice]

error: 'class XArray<unsigned char>' has no member named 'insert'; did you mean 'Insert'?

error: 'class XArray<unsigned char>' has no member named 'end'

error: 'const class XArray<unsigned char>' has no member named 'empty'; did you mean 'Empty'?

error: 'const class XArray<unsigned char>' has no member named 'size'; did you mean 'Size'?

[Jief_Machak]

41 minutes ago, Slice said:

error: 'class XArray<unsigned char>' has no member named 'insert'; did you mean 'Insert'?

error: 'class XArray<unsigned char>' has no member named 'end'

error: 'const class XArray<unsigned char>' has no member named 'empty'; did you mean 'Empty'?

error: 'const class XArray<unsigned char>' has no member named 'size'; did you mean 'Size'?

The api of XArray are not the same as vector. It just serves the same purpose. But feel free to rename method if you'd like it to look similar more to vector.

Add an object to the end is just Add (might that would more clear if renamed to Append).

insert is Insert.

The capital at the beginning of the method name is not a convention anymore. I can rename everything method name starting with a lower case.

There is no 'end'. There is not iterator.

Size() is the size allocated, Length() is the actual number of element.

I think we should rename Size allocatedSize, because of STL where size is the actual number of element, a lot of mistake can happen if we keep that name.

[Slice]

So it will be good to rename Size() -> allocatedSize(), Length() -> size()?

[Slice]

May be implement .end() as [length-1]?

[Slice]

I committed my propositions but it is not working. Something is not good.

But I tested in QEMU compiled by VS so it may be other problems.

 

EDITED: No, it works!

New lodepng as cpp works!

[Jief_Machak]

 

4 hours ago, Slice said:

So it will be good to rename Size() -> allocatedSize(), Length() -> size()?

yes. We can keep length() and size() return the same thing, or just size().

3 hours ago, Slice said:

May be implement .end() as [length-1]?

In STL, end is an iterator. We don't use them here, so I wouldn't define something called the same. But why having a convenience method lastElement() ?

[Jief_Machak]

15 hours ago, Pene said:

There was a mistake in memory allocation of devtree nodes.

sizeof(__typeof_am__(*Child)) will give 8 while sizeof(DEV_PROPERTY) is much more than that. This must result in all kind of weird errors.

Please try again after my last commit.

Very well spotted ! Bravo !

 

May I continue to insist avoiding using explicit type.

The mistake was "sizeof(__typeof_am__(*Child))" instead of "sizeof(**Child)". "sizeof(**Child)" is better than "sizeof(DEV_PROPERTY)" because if **Child type change, "sizeof(**Child)" still works.

 

But because we are now using C++ compiler, we can do "*Child = new (__typeof_am__(**Child));", which is even better.

 

[Slice]

29 minutes ago, Jief_Machak said:

 

yes. We can keep length() and size() return the same thing, or just size().

In STL, end is an iterator. We don't use them here, so I wouldn't define something called the same. But why having a convenience method lastElement() ?

I found it is simpler to modify lodepng to not use .insert() and .end()

But I need empty() return bool.

as in STL

    bool empty() const
        {    // test if sequence is empty
        return (this->_Mysize == 0);
        }

[Jief_Machak]

You're welcome to modify Empty method (rename it to empty?), but you can't declare a method cont that modify its object.

I'm not sure why you would return a bool. It will be always true ? always false ?

I'm thinking, do you want a method that checks if it's empty or not ? In that case, I think "isEmpty" is a better name. This will be const, right.

just add this :

const bool isEmpty() const { return size() == 0; }

 

The current Empty() actually empty the array. Would you prefer setEmpty(), or removeAllElements() ?

[Slice]

I prefer setEmpty and isEmpty

[Jief_Machak]

Just now, Slice said:

I prefer setEmpty and isEmpty

agreed.

[Slice]

committed

[Slice]

It is not working again at the same place.

[Jief_Machak]

6 minutes ago, Slice said:

It is not working again at the same place.

I put a DBG("%d",sizeof(**Child)) I got 104 (if I remember well). Same as sizeof(DEV_PROPERTY)).

Then I used new. I put a break point im new operator and count was also 104.

But I didn't check VS2017 yet for the same reason. C++ Globals not implemented, so VS cannot work.

I'll check again in a minute.

[Pene]

When I traced it yesterday the hang was at this loop (the code that was freeing the allocated DEV_PROPERTY nodes).

There should not be difference between sizeof(**Child) and sizeof(DEV_PROPERTY), but I'm not sure about if freeing works the same when using new.

[Slice]

I checked with GCC53. Hang using "new".

Then I replaced back to

                    *Child = (__typeof_am__(*Child))AllocateZeroPool(sizeof(**Child));
                    DBG("created *Child sizeof %d\n", sizeof(**Child));
                    
            //        *Child = new (__typeof_am__(**Child));

and it works.

log contains

5:252  0:001  created *Child sizeof 104

[Jief_Machak]

I'm sorry, went too fast : yes we have to use delete instead of FreePool, if we allocate with new.

Leave it like this. I'll test new-delete combination later.