Jump to content
823 posts in this topic

Recommended Posts

I understand. Moving this commit "more type casting fixes" to a branch will allow us to easily spot all the modifications made for VC compatibily and check them when it'll be time to merge back to master.

For example, the bug introduce here "**ListPtr = AllocatePool(sizeof(VOID *) * AllocateCount);" might be forgotten if we leave it in master.

 

Also changing "char newname[AsciiStrLen(devprop_list.name)+1];" by "char* newname = (char*)AllocatePool(AsciiStrLen(devprop_list.name)+1);" might be not a so good move (it's slower).

 

Casting CONST to non CONST, here "gRT->SetVariable((CHAR16*)KbdPrevLang, &gEfiAppleBootGuid, Attributes, LangLen, &gSettings.Language);" could lead to problem. Instead, the prototype of the function should modified. And I don't understand why it was needed when it compiled fine with clang and gcc.

 

Do all VC changes in master branch will make then harder to review... And the master branch won't compile anymore while it's in progress. Branches were invented for exactly that !

The first thing should be fixed. The second is correct, you should not dynamically allocate on the stack, and is not allowed in C90, which is what vs uses by default and I think the edk2 expects. The stack may not even be big enough to do that. The third thing cannot be done at all, the cast must happen, that prototype is specified in UEFI and cannot be changed.

3 minutes ago, Jief_Machak said:

My point wasn't to discuss these modifications. My point is to convince to do them in a branch so it will be easily reviewed.

 

OK, I just not easy in git yet. It is simpler for me do all in one place.

 

Anyway new Clover is not working crashing after

6:345  0:011          Result of bootcode detection: bootable FreeDOS (freedos)
0:100  0:100  MemLog inited, TSC freq: 2591697500
Will debug this issue.

Looking again to the error produced with VS  "for '&' lvalue required".

This procedure

ParseLoadOptions (

                  OUT  CHAR16** Conf,

                  OUT  TagPtr* Dict

                  )

define first argument as pointer to pointer or array.

The construction

*Conf = (__typeof__(*Conf))AllocateZeroPool ((TailSize + 1) * sizeof (CHAR16));

causes the error with VS2017.

It looks like decltype will be applied only for structures not for pointer to structures.

So why I had to replace such definitions by explicit

*Conf = (CHAR16*)AllocateZeroPool ((TailSize + 1) * sizeof (CHAR16));

then it works.

 

13 minutes ago, apianti said:

You need some other stuff implemented for decltype to work but it works for pointers (it's mainly used in templates with pointers to parameterized types), and __typeof__ is compiler specific to gcc/clang.

I did this

#ifdef _MSC_VER

#define __typeof__(x) decltype(x)

#endif

They are not equivalent operator keywords. They behave slightly differently, in that decltype gives references in many cases, so you probably have to use something like static_cast<decltype(...)>(...).

 

https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Typeof.html

https://en.cppreference.com/w/cpp/language/decltype

 

EDIT: Actually, I think that might fail type checking. So probably need to really use reinterpret_cast<decltype(...)>(...).

Edited by apianti
3 hours ago, Slice said:

OK, I just not easy in git yet. It is simpler for me do all in one place.

 

Anyway new Clover is not working crashing after

6:345  0:011          Result of bootcode detection: bootable FreeDOS (freedos)
0:100  0:100  MemLog inited, TSC freq: 2591697500
Will debug this issue.

No success. :(

It may hang in any moment while scanning volumes for boot sector or for nvram.plist. There is no definite code for crash...

Ok, so I defined of __typeof_am__ to be able to get type of array member without adding a reference (that's why we get that error message about & and lvalue).

 

I also found why my windows script didn't work. It was because of () in echo message in a .bat. It's commited.

 

I'm finished for today. Tomorrow we will make it compile and run !

 

Good night.

15 hours ago, Jief_Machak said:

 

Casting CONST to non CONST, here "gRT->SetVariable((CHAR16*)KbdPrevLang, &gEfiAppleBootGuid, Attributes, LangLen, &gSettings.Language);" could lead to problem. Instead, the prototype of the function should modified.

Returning to this problem. I agree to modify prototypes and use CONST CHAR in Clover codes.

Strange but native EDK2 contains IN CHAR16 while we need IN CONST CHAR16. Somehow EDK2 developers didn't bother about this.

For testing purpose I will change this in my local sources  but then I can stash it.

CC_FLAGS works. If flags are the same for CC and CXX, no need to define both.

Don't know why it's not committed... My bad.

Did you see this commit "Paranthesis in echo command doesn't work."? I don't understand how nobody has that problem before ???

Because the script was changed by someone who hard coded a bunch of values that are different on every system and modified the script to work differently because I'm guessing they didn't know they only had to define some environment variables? I just modify it locally myself now because someone kept changing it back after every time I fixed it.

×
×
  • Create New...