C++ proposition

[Jief_Machak]

You were already trying the enum way :-)...

[Slice]

now I will commit conflicting sources

[Jief_Machak]

That's ok, I can resolve them. But don't commit in the meantime.

[Jief_Machak]

Done. It's compiling, but there is unresolved external.

 

Can have a look later, if you'd like.

[Jief_Machak]

I fixed the unresolved. Mainly, it was because when you move a function in a class (becoming a method), you forgot to remove the old prototype in .h

Well, I don't have unresolved external when I compile with Eclipse. But I have some when I compile with ebuild.

[Jief_Machak]

Ok, I understood. You excluded pointer.cpp, which is not used anymore. I was still compiling with it.

 

I had to pull out of XPointer the function TimeDiff because it's used by screen.cpp outside a XPointer. Anyway, it doesn't really make sense to define a method that doesn't use any member of the class it's define in. TimeDiff is more a basic tool (or platform tool?) that could be defined in a file (Platform.cpp, or ?) and included in Platform.h. Like StrCmp, StrCpy etc.

Because Platform may already be too big, we can start a .cpp/.h files named something like BasicTools, PlatformTools, LowlevelTools, CUtils... Platform.h would include that new header.

 

[Slice]

I was working parallel to you and resolved most of this issues. I committed them.

I got full compilation and linking with GCC53. Tomorrow will check with VS.

Now I think we can switch from char16 and EG_IMAGE to XStringW and XImage.

 

I will revise theme support. It looks like to be a separate class.

 

[Jief_Machak]

You removed const on "  CHAR16      *Title;  //Title is not const. It can be dynamically changed" in REFIT_MENU_SCREEN.

 

const CHAR16* Title doesn't mean you can't change the title. const only apply to the pointee, not the pointer. So you can still change where Title point.

 

In fact, I put "const" back and it's compiling fine, without warning. I agree, changing to XStringW will end the discussion :-)

[Slice]

The crash is at

      if (MainEntry.getLEGACY_ENTRY()) {

[Jief_Machak]

I changed "  Entry = (__typeof__(Entry))AllocateZeroPool(sizeof(LOADER_ENTRY));"

by "//  Entry = new LOADER_ENTRY();" 

 

and now it's back to AllocateZeroPool.

It's not possible to use AllocateZeroPool with objects that has a vtable (some virtual function).

[Slice]

git push?

[Jief_Machak]

Yes, in a minute. 

[Jief_Machak]

Sorry, forgot to say it's done.

[Slice]

I see.

Now I am testing VS compilation. It's a pity there are memset generation at

new LOADER_ENTRY();

new LEGACY_ENTRY();

new REFIT_MENU_ENTRY_LOADER_TOOL();

new REFIT_MENU_ENTRY_CLOVER();

[Jief_Machak]

What do you mean by memset generation ?

I forgot the () ? Should work the same because it's an object with ctor.

[Slice]

27 minutes ago, Jief_Machak said:

What do you mean by memset generation ?

I forgot the () ? Should work the same because it's an object with ctor.

I see linker warnings and disassembler

; 153  :   DuplicateEntry = new LOADER_ENTRY();

  00020    b9 e8 00 00 00     mov     ecx, 232        ; 000000e8H
  00025    e8 00 00 00 00     call     ??2@YAPEAX_K@Z        ; operator new
  0002a    48 8b d8     mov     rbx, rax
  0002d    48 85 c0     test     rax, rax
  00030    74 45         je     SHORT $LN5@DuplicateL
  00032    33 d2         xor     edx, edx
  00034    41 b8 e8 00 00
    00         mov     r8d, 232        ; 000000e8H
  0003a    48 8b c8     mov     rcx, rax
  0003d    e8 00 00 00 00     call     memset
  00042    0f 57 c0     xorps     xmm0, xmm0
 

[Slice]

4 hours ago, Slice said:

The crash is at

      if (MainEntry.getLEGACY_ENTRY()) {

Windows QEMU with VS compilation crashes exactly at the same place.

[Jief_Machak]

3 minutes ago, Slice said:

Windows QEMU with VS compilation crashes exactly at the same place.

You remember that C++ globals still don't work with VS. Some menu and menuitem are globals.

Indeed, I'm currently preparing to have a look now.

[Jief_Machak]

I'm wondering : you should have "A test failed".

Did you have it ?

[Slice]

1 minute ago, Jief_Machak said:

I'm wondering : you should have "A test failed".

Did you have it ?

Yes, with VS "a test failed".

Anyway the crash

 

Quote

 

The crash is at

      if (MainEntry.getLEGACY_ENTRY()) {

 

 

happen in macOS with GCC53 compilation.

[Slice]

The problem is

REFIT_ABSTRACT_MENU_ENTRY is not REFIT_MENU_ITEM_ABSTRACT_ENTRY_LOADER required for virtualization.

[Slice]

If I make this

  // Ignore this loader if it's device path is already present in another loader
//  if (MainMenu.Entries) {
  DBG("Entries=%d\n", MainMenu.Entries.size());
    for (UINTN i = 0; i < MainMenu.Entries.size(); ++i) {
      REFIT_ABSTRACT_MENU_ENTRY& MainEntry = MainMenu.Entries[i];
      DBG("entry %lld %a\n", i, (MainEntry.SubScreen == nullptr)?"empty":"full");
      // Only want legacy
//      if (MainEntry && (MainEntry->getLEGACY_ENTRY())) {
      if (MainEntry.isLegacy) {
        DBG("a1\n");
        REFIT_MENU_ENTRY& M1 = reinterpret_cast<REFIT_MENU_ENTRY&>(MainEntry);
        DBG("a10\n");
        REFIT_MENU_ITEM_ABSTRACT_ENTRY_LOADER& M2 = reinterpret_cast<REFIT_MENU_ITEM_ABSTRACT_ENTRY_LOADER&>(M1);
        DBG("a11\n");
        LEGACY_ENTRY& M3 = reinterpret_cast<LEGACY_ENTRY&>(M2);
        DBG("a12\n");
        if (StriCmp(M3.DevicePathString, Volume->DevicePathString) == 0) {
          DBG("a13\n");
          return  true;
        }
      }
    }
//  }

 

Then it somehow works 

And then I got a stop at Anime which is an another story.

[Jief_Machak]

MainEntry->getLEGACY_ENTRY() is perfectly valid for all menu item.

Please wait instead modifying too much. I'm on it and I use GDB, which is easier to find these problem.

[Jief_Machak]

You just put back again the "AllocateZeroPool" for allocating objects !!!

Please don't : it CAN'T work for objects with virtual method. Of course it crashed.

[Slice]

I will stash my codes.

I think there is something like wrong virtual table.