C++ proposition

[Slice]

It is good array size calculation?

STATIC CONST XString LinuxInitImagePath[] = {
   "initrd%s"_XS,
   "initrd.img%s"_XS,
   "initrd%s.img"_XS,
   "initramfs%s"_XS,
   "initramfs.img%s"_XS,
   "initramfs%s.img"_XS,
};
STATIC CONST UINTN LinuxInitImagePathCount = (sizeof(LinuxInitImagePath) / sizeof(CHAR16 *));

sizeof(CHAR16*) should be sizeof(XString*)? 

Or full rewrite the procedure to search and compare linux entries? Childrens codes...

[Slice]

Can I use XString as an unicode array?

XString str3;

str3.takeValueFrom(L"Выход");

 

What will be str3[1]? "ы"?

[Slice]

Huh!

5107:
0:590 0:000 MSR 0xCE 00080838_F1012A00

5109:
0:100 0:000 MSR 0xCE 00080838_80838F1012A00
 

 

[Jief_Machak]

44 minutes ago, Slice said:

Can I use XString as an unicode array?

XString str3;

str3.takeValueFrom(L"Выход");

 

What will be str3[1]? "ы"?

No, problem is you get the 1 utf8 char, that'll probably be a surrogate.

I've planned to write a method in XString which give "reconstructed" char one by one. If you need it, wait a sec...

 

4 hours ago, Slice said:

It is good array size calculation?

STATIC CONST XString LinuxInitImagePath[] = {
   "initrd%s"_XS,
   "initrd.img%s"_XS,
   "initrd%s.img"_XS,
   "initramfs%s"_XS,
   "initramfs.img%s"_XS,
   "initramfs%s.img"_XS,
};
STATIC CONST UINTN LinuxInitImagePathCount = (sizeof(LinuxInitImagePath) / sizeof(CHAR16 *));

sizeof(CHAR16*) should be sizeof(XString*)? 

Or full rewrite the procedure to search and compare linux entries? Childrens codes...

Yes, you're right. 

But it should be sizeof(XString)

10 minutes ago, Slice said:

0:100 0:000 MSR 0xCE 00080838_80838F1012A00

Not sure what that is, but maybe a problem introduced with snprintf ?

[Slice]

Format %08llX printed 16 hex characters

MsgLog("MSR 0xCE              %08llX_%08llX\n", (msr>>32), msr);

[Slice]

What is the difference between XObjArray and XArray? Why I have to prefer XObjArray?

[Jief_Machak]

Even though you declare XObjArray<Class myObj>, XObjArray manages an array of pointer to Class. Which allow to remove one element, add one etc. very quickly. When an element is remove, object is freed and the hole in the array is removed by memmove.

 

With a standard array of object, XArray<Class myObj> or Class myObjArray[x], you can't free an object in the middle of the array. You are not supposed to do, for example, delete myObjArray[2], because is NOT a pointer allocated by new.

Also, if you extend the array memory with realloc, ctor of new objects are not called.

 

Rule is :

  - if you have a fixed size array of object you'll never have to free or to move (like SelectionImages), Class myObjArray[x] is fine, even better than XObjArray<Class myObj>, but both works.

  - if you want to be able to add and remove objects in the array XObjArray<Class myObj> is the way to go. Using XArray<Class myObj> NEVER works because ctor won't be called.

 

 

[Slice]

So XArray can be used only for non-class types?

[Jief_Machak]

Exactly. Its purpose is to check indexes when accessed and ofter few convenience method for extending/shrinking

[Slice]

I checked unicode strings

sizeof("Нечто") == 11

strlen("Нечто") == 10

StrLenInWChar("Нечто") == 5

Good! I have to use it.

[Jief_Machak]

Yes, but wait a bit if you can, because UTF16 can also have surrogate. Which means that "StrLenInWChar" will give you more than the number of char.

I'll commit soon a strlenInUnicodeChar and a getUnicodeCharAt (or something equivalent) that will properly handle UTF16 surrogate pairs.

[Slice]

But XStringW has no such method. 

Can I make this moment this?

TextLength = StrLenInWChar(Text.c_str()); //it must be UTF16 length

where Test is XStringW

 

[Jief_Machak]

If Text is a XStringW, you'll get the same answer as doing Text.size().

[Slice]

I got different results with

c = Text [ inx ];

and

c = Text.wc_str() [ inx ];

so I bothered to rebuild this part.

It looks like CHAR16* Str contains UTF8 characters even if it can be UTF16.

[Jief_Machak]

They wouldn’t be valid utf8 if embedded in char16.

it should not be different, if it is, there is a bug to find.

how different is it ?

Are you using VS ? Because their utf16 literals are wrong.

For VS, you’ll have to convert an utf8 littéral with XString().takeValueFrom()

[Jief_Machak]

2 hours ago, Slice said:

I got different results with

c = Text [ inx ];

and

c = Text.wc_str() [ inx ];

so I bothered to rebuild this part.

It looks like CHAR16* Str contains UTF8 characters even if it can be UTF16.

 

I added a small test in XStringW :

		XStringW utf16;
		utf16.takeValueFrom(L"Выход из подменю, обновление главного меню\n");
		for ( size_t i = 0 ; i < utf16.length() ; i++ ) {
			if ( utf16[i] != utf16.wc_str()[i] ) {
				return 100;
			}
		}

that shows that this :  Text.wc_str() [ inx ];  and that  Text.wc_str() [ inx ]; are the same. So the problem is elsewhere...

[Slice]

I am also exploring this bug

See PartNumber and SerialNumber of two DIMMs. They are were similar with old Clover. And the SerialNumber have to be char8 as in debug.log

0:100  0:000  Type 16 Index = 0
0:100  0:000  Total Memory Slots Count = 2
0:100  0:000  Type 17 Index = 0
0:100  0:000  SmbiosTable.Type17->Speed = 1600MHz
0:100  0:000  SmbiosTable.Type17->Size = 4096MB
0:100  0:000  SmbiosTable.Type17->Bank/Device =  DIMM A
0:100  0:000  SmbiosTable.Type17->Vendor = Samsung
0:100  0:000  SmbiosTable.Type17->SerialNumber = 0094EA68
0:100  0:000  SmbiosTable.Type17->PartNumber = M471B5273DH0-CK0  
0:100  0:000  Type 17 Index = 1
0:100  0:000  SmbiosTable.Type17->Speed = 1600MHz
0:100  0:000  SmbiosTable.Type17->Size = 4096MB
0:100  0:000  SmbiosTable.Type17->Bank/Device =  DIMM B
0:100  0:000  SmbiosTable.Type17->Vendor = Samsung
0:100  0:000  SmbiosTable.Type17->SerialNumber = 9383A4AD
0:100  0:000  SmbiosTable.Type17->PartNumber = M471B5173DB0-YK0  
 

[Pene]

Hi,

 

As I have no experience with C++, could you please advise what's a more elegant solution to initializing to an empty Image?

You can see what I mean in this commit (had no better idea): https://github.com/CloverHackyColor/CloverBootloader/commit/5eadae5eb85b3e3c7653701e5bb1df0832ebe498

 

P.S. Feel free to commit a better way to do that  

Edited April 12, 2020 by Pene

[Slice]

8 hours ago, Pene said:

Hi,

 

As I have no experience with C++, could you please advise what's a more elegant solution to initializing to an empty Image?

You can see what I mean in this commit (had no better idea): https://github.com/CloverHackyColor/CloverBootloader/commit/5eadae5eb85b3e3c7653701e5bb1df0832ebe498

 

P.S. Feel free to commit a better way to do that  

Instead 

    if (!Entry->Image.isEmpty()) { 
       XImage EmptyImage;
       Entry->Image     = EmptyImage;
     }

You have to write

Entry->Image.setEmpty();

[Jief_Machak]

Think of an objet as a type. Almost like a native type.

BUT objects have a default initialisation. Native types does not.

 

As for native types, you don't need to check previous value to assign a new one. So the check "if (!Entry->Image.isEmpty())" is not needed.

 

This :

       XImage EmptyImage;
       Entry->Image     = EmptyImage;

is the same as 

	Entry->Image     = XImage();

And that would work the same as i = 0; :

  - an new empty object is created

  - a copy is made in Entry->Image

  - new object is destroy

 

An other difference is that native doesn't generate dynamic allocation. So writing 0, which "create" an int, doesn't really cost. "Destroying" a native type is just not using it anymore, so no cost. An object can be costly to create and destroy, that's why a setEmpty method is a way to be sure that no temporary object is used.

 

 

 

 

 

[Pene]

@Slice

 

I see I partially overwritten your fix of VectorGraphics crash.

But I think the fix I committed is simpler. If you compare the state that was before your fix to what I committed, my only change in VectorGraphics.cpp is:

NSVGparser *mainParser = (NSVGparser*)SVGParser;
...
mainParser = nsvgParse((CHAR8*)buffer, 72, 1.f); 

to

NSVGparser *mainParser;
...
SVGParser = (void *)nsvgParse((CHAR8*)buffer, 72, 1.f);
mainParser = (NSVGparser*)SVGParser;

What basically was happening that it wasn't modifying the SVGParser pointer when it was first assigned to another variable and then set through that variable.

So it wasn't needed to reintroduce passing SVGParser by parameter, all needed was to fix this assignment.

 

P.S. I see you left XCINEMA=1, seems to work ok, but something is still wrong with positioning for legacy anime themes (check Christmas for example).

Edited April 16, 2020 by Pene

[Jief_Machak]

Now, Platform.h is a meta-header that includes basic need for all modules. It's supposed to contains only includes.

DO NOT define anything in there. I know it seems handy because it's instantaneously shared with all, but please don't. If it make sense to share something with all modules (macro, extern var or function), define it in the module it belongs to and create an include in Platform.h.

 

The point is to have modules that makes sense. A module is a bit like a C++ object. Everything is defined in the couple .h .cpp, then other modules include the header and start using it.

 

Everyone's ok with that ?

oops, forgot to update refit. Wait a sec before pulling.

[Jief_Machak]

refit.inf updated.

I didn't test VS compilation, but I just moved things around. Hope it's ok.

[Pene]

6 minutes ago, Jief_Machak said:

refit.inf updated.

I didn't test VS compilation, but I just moved things around. Hope it's ok.

Hi, reported revision is broken (gcc Mac build)

It shows Clover 65535.

Edited April 16, 2020 by Pene

[Jief_Machak]

I'm on it.

First : which Version.h is the right one? Some files includes the one in the main dir and main.cpp include the one in rEFIt_UEFI.