Slice Posted February 20, 2020 Share Posted February 20, 2020 Few fixes and GCC53 compilation is also working. Committed. Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 20, 2020 Author Share Posted February 20, 2020 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 ! Link to comment Share on other sites More sharing options...
apianti Posted February 20, 2020 Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
Slice Posted February 20, 2020 Share Posted February 20, 2020 char newname[AsciiStrLen(devprop_list.name)+1];" is impossible because array must be defines with constant size. It may be a Constructor to do the work but char is a simple type. Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 20, 2020 Author Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
apianti Posted February 20, 2020 Share Posted February 20, 2020 Didn't see me say anything about that, I also believe that this should be done when using git. Link to comment Share on other sites More sharing options...
Slice Posted February 20, 2020 Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
Slice Posted February 20, 2020 Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
apianti Posted February 20, 2020 Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
Slice Posted February 20, 2020 Share Posted February 20, 2020 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 Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 20, 2020 Author Share Posted February 20, 2020 looks like decltype doesn't work for void** in VC. This works "*ListPtr = (void**)AllocatePool(sizeof(VOID *) * AllocateCount);" Link to comment Share on other sites More sharing options...
apianti Posted February 20, 2020 Share Posted February 20, 2020 (edited) 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 February 20, 2020 by apianti Link to comment Share on other sites More sharing options...
Slice Posted February 20, 2020 Share Posted February 20, 2020 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... Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 20, 2020 Author Share Posted February 20, 2020 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. Link to comment Share on other sites More sharing options...
Slice Posted February 21, 2020 Share Posted February 21, 2020 7 hours ago, Jief_Machak said: It's commited. Don't forget to push. Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 21, 2020 Author Share Posted February 21, 2020 1 hour ago, Slice said: Don't forget to push. I did forget. Done. Link to comment Share on other sites More sharing options...
Slice Posted February 21, 2020 Share Posted February 21, 2020 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. Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 21, 2020 Author Share Posted February 21, 2020 Don't worry, I'll take of that later. I'm working on making the C++ globals works in MSVC. Link to comment Share on other sites More sharing options...
Slice Posted February 21, 2020 Share Posted February 21, 2020 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. Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 21, 2020 Author Share Posted February 21, 2020 If you change any edkII prototype, use JCONST. This way, modules outside rEFIt will still compile as before. Link to comment Share on other sites More sharing options...
Slice Posted February 21, 2020 Share Posted February 21, 2020 Where I have to #define JCONST CONST In what module? Also many of these functions are called from BIOS UEFI. Will it raise any problem? Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 21, 2020 Author Share Posted February 21, 2020 I defined JCONST in compiler option in refit.inf. Nothing to do. If you use another build system, you have to define it. No problem for the rest of the BIOS. Link to comment Share on other sites More sharing options...
Slice Posted February 21, 2020 Share Posted February 21, 2020 This way MSFT:*_*_*_CC_FLAGS = /Os /wd4201 /D JCONST=const or may be define MSFT:*_*_*_CXX_FLAGS = /Os /wd4201 /D JCONST=const Link to comment Share on other sites More sharing options...
Jief_Machak Posted February 21, 2020 Author Share Posted February 21, 2020 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 ??? Link to comment Share on other sites More sharing options...
apianti Posted February 21, 2020 Share Posted February 21, 2020 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. Link to comment Share on other sites More sharing options...
Recommended Posts