Clover General discussion

[SavageAUS]

Clover 4214 builds fine with Xcode 9 and build_clover 4.5.4 + previous fixes. Great work guys.

 

Have all previous fixes been pushed to source?

tools_def and UefiLib.h

[HmO]

where new link download Clover 4211.

 

Thank

 

Clover 4218

[Slice]

Clover 4214 builds fine with Xcode 9 and build_clover 4.5.4 + previous fixes. Great work guys.

 

Have all previous fixes been pushed to source?

tools_def and UefiLib.h

UefiLib.h is in Clover sources now.

tools_def I see no reason to change.

[ccphuc2016]

Clover 4214

THANK

[mhaeuser]

EDK2 developers just don't care about clang code requirement. See stackoverflow

So I need again correct them.

Sorry, what did you fix? All I see is a new, apparently unused parameter.

[Slice]

Sorry, what did you fix? All I see is a new, apparently unused parameter.

See stackoverflow explanations and tricks.

Instead of (...) you must explicitly show first parameter (void* a, ...) to avoid underfined behavior.

So 

 VariableGetBestLanguage (
   IN CONST CHAR8  *SupportedLanguages, 
   IN BOOLEAN      Iso639Language,
+  IN CONST CHAR8  *Lang,
   ...
   )
 {

which is first parameter in variable list, and

-  VA_START (Args, Iso639Language);
+  VA_START (Args, Lang);

because Boolean is not a parameter from the list. The list must contains strings languages.

 

Best wishes!

 

 

3  

Clang is right. See [support.runtime]/3. – Kerrek SB Dec 3 '15 at 15:32

[mhaeuser]

Instead of (...) you must explicitly show first parameter (void* a, ...) to avoid underfined behavior.

Yes, you need at least one parameter, GetBestLanguage() even has two.

 

because Boolean is not a parameter from the list. The list must contains strings languages.

I do not see why that would matter, neither the stackoverflow page you linked, nor any site regaridng VA args I quickly read suggest it.

 

Actually, you even broke the code because the second parameter of va_start() must be the argument preceeding the variable argument list. Doing it the way you do it now drops the first member of the list, which is now the unused parameter 'Lang'.

 

Would you mind sharing the error/warning that made you change the code?

 

EDIT: Talked to vit9696 and yes, your intention is right, the standard is not a piece of cake to read... anyway, if you want to make it work, change the loop to a do-while and use Lang as first member of the list.

[Slice]

You may not use my patches if you didn't understand them.

 

 the unused parameter 'Lang'.

 

 

 

->

-  VA_START (Args, Iso639Language);
+  VA_START (Args, Lang);

Do you see?

[mhaeuser]

*sigh*

Thanks, I understand your "fix" well enough, I was just unsure about something from the specification, which was not related to your patch being wrong, but for the edk2 code to be wrong in the first place.

 

You pass Param1, Param2, Param3, Param4, Param5.

Which map to: SupportedLanguages, Iso639Language, Lang and the first two members of the VA list.

A real life example would be: SupportedLanguages = "en-usde-de", Iso649Language = FALSE, Lang = "en-us", VA1 = "de-de", VA2 = "fr-fr"

 

Now, what happens when running your patch?

1. "VA_START (Args, Lang)" opens the VA list and the beginning of the loop

2. "while ((Language = VA_ARG (Args, CHAR8 *)) != NULL)" - VA_ARG() returns VA1, i.e. "de-de"

3. On the next interation, the same function returns "fr-fr"

 

Where did "en-us" go? Might it not be used, because you never use the value of Lang? RIP "en-us", you will never be forgotten...

 

And yes, I will not use your patches, not because I don't understand them, but because I prefer working code.

[Slice]

Agree, first argument will be lost

#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>

#define IN
#define CONST const
#define CHAR8 char
#define BOOLEAN unsigned char

#ifndef VA_START
// Use the native arguments for tools.
#define VA_START va_start
#define VA_ARG   va_arg
#define VA_END   va_end
#define VA_LIST  va_list

#endif


CHAR8 *
VariableGetBestLanguage (
                         IN CONST CHAR8  *SupportedLanguages,
                         IN BOOLEAN      Iso639Language,
                         IN CONST CHAR8  *Lang,
                         ...
                         )
{
  CHAR8        *Language;
  VA_LIST      Args;
  VA_START (Args, Lang);
  while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) {
    printf("%s ", Language);
  }
  VA_END (Args);
  return ("\nHello!\n");
}

int main(int argc, char ** argv)
{
  printf("%s\n", VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr"));
	return 0;
}
 

->

iMac-2:TestC slice$ ./test
de-de fr-fr fr-fr 
Hello!

But why fr-fr is double?

Revert my patch and use -Wno-varargs

OK, it works

 

iMac-2:TestC slice$ clang test.c -o test1 -Wno-varargs

iMac-2:TestC slice$ ./test1

en-us de-de fr-fr fr-fr 

Hello!

 

But the clang is right. This is wrong code style.

 

I can do better

iMac-2:TestC slice$ clang test.c -o test2
iMac-2:TestC slice$ ./test2
en-us de-de fr-fr 
Hello!

[RehabMan]

But why fr-fr is double?

The answer to your question is in the documentation/source code comments.

 

 

 

@param[in] ... A variable argument list that contains pointers to

Null-terminated ASCII strings that contain one or more

language codes in the format specified by Iso639Language.

... The variable argument list

is terminated by a NULL.

 

 

In other words, your test case is bugged.

You wrote:

VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr")

 

Should be:

VariableGetBestLanguage("en-usde-de", 0, "", "en-us", "de-de", "fr-fr", NULL)

 

Note: My fix was to change "IN BOOLEAN Iso639Language," to "IN UINTN Iso639Language,". 

Based on my understanding of the compiler's complaint, it should be ok.

 

[RehabMan]

yes. I agree your code method is better - no warnings or overrides!

 

with a slight code mod this example proves it without a doubt - Iso639Language value is passed properly

 

....

{

  CHAR8        *Language;

  VA_LIST      Args;

//  VA_START (Args, Iso639Language);

  VA_START (Args, Lang);

  while ((Language = VA_ARG (Args, CHAR8 *)) != NULL) {

    printf("%s ", Language);

    if (Iso639Language) printf("T ");

                   else printf("F ");

  }

  VA_END (Args);

  return ("\nHello!\n");

}

int main(int argc, char ** argv)

{

  printf("%s\n", VariableGetBestLanguage("en-usde-de", 1, "", "en-us", "de-de", "fr-fr",NULL));

return 0;

}

 

$ a.out

en-us T de-de T fr-fr T

Hello!

 

2cents more... this way seems to me a bit more straight forward of a fix. 

 

You are still not understanding the intent of the original edk2 code.

The edk2 code is correct.

The compiler is complaining about a minor standards compliancy issue that does not affect the platform we are coding to.

But, as I mentioned, it is easy to avoid the warning by using a type for the vararg anchor that is not subject to promotion.

[tluck]

You are still not understanding the intent of the original edk2 code.

The edk2 code is correct.

The compiler is complaining about a minor standards compliancy issue that does not affect the platform we are coding to.

But, as I mentioned, it is easy to avoid the warning by using a type for the vararg anchor that is not subject to promotion.

 

 

I do understand. i reverse my course - i am in agreement with you. using unsigned long (UINTN in edk2) keeps original code intent and avoids compiler nuisance.

[Slice]

The compiler do not warning variable size difference? BOOLEAN UINTN

 

The edk2 code is correct.

 

For gcc (linux) or VS (Windows). But they are not care about clang.

We may just wait while someone send a message to edk2 list with the claim and someone resolves it.

[mhaeuser]

Note: My fix was to change "IN BOOLEAN Iso639Language," to "IN UINTN Iso639Language,".

Based on my understanding of the compiler's complaint, it should be ok.

Imo Slice's attempt is the better one because all types stay the same then, he would just need to turn thewhile-into a do-while-loop and use Lang on the first iteration.

 

Clang thinks passing an argument that is default-argument-promoted to va_start is UB.. I read the spec and honestly, idk, when you're not used to all tge expressions, it's a pain to read. Basically depends on whether char is "compatible with" int, if I found the part Clang refers to.

 

In regards to C11, it seems like the arg preceeding the VA needs to be of a compatible type with the one of the following var as well.

 

I'm on phone, so can't check right now, but may you pass nothing to a VA list? If so, keeping "Lang" around would enforce at least passing one arg, which is good. In regards to C11 (or maybe even C99, the spec is a pain), literally defining the first member of the VA might be beneficial, idk.

 

EDIT: char is not compatible with int and that is definitely an(the?) issue, but *MIGHT* not be the only one. It definitely isn't when C11 compliancy is a concern... thx vit

 

EDIT2: Comments on C11 were false.

[RehabMan]

he would just need to turn thewhile-into a do-while-loop and use Lang on the first iteration.

I thought of that as well, but wanted to avoid changing the core logic so much.

[Gogeta5026]

It took a 2-3 days but I figured out that...

In Clover
If I have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:
1) Boot into Windows and Linux without any problems

2) I CAN NOT boot into High Sierra in any way, my screen is corrupt

High Sierra with CsmVideoDxe-64.efi in my drivers64UEFI folder

 

If I DONT't have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:
1) Boot into High Sierra without any problems

2) I CAN NOT boot into Windows and Linux

Linux and Windows without CsmVideoDxe-64.efi in my drivers64UEFI folder (just a cursor)

MY QUESTION IS...
Is there a solution to this problem? Can I keep CsmVideoDxe-64.efi in my folder so, Windows and Linux will boot and disable it from High Sierra? Should I just take CsmVideoDxe-64.efi out but... How do I get into Windows and Linux through Clover then? Is there another way to fix this problem???
 

I have:

Intel Core i7 7700K 4.2 Ghz

Gigabyte GA-Z270MX/Gaming 5

Samsung CF791 Series 34-Inch Curved Widescreen Monitor (C34F791)  3440x1440 4K

Samsung U28E590D 28-Inch UHD LED-Lit Monitor with Freesync support  3840x2160 4K

AMD Radeon Pro Vega Air-Cooled Frontier Edition 16 GB

ANY HELP WOULD BE GREAT! I attached my config.plist if that helps in any way for somebody.

config.plist.zip

[mhaeuser]

I thought of that as well, but wanted to avoid changing the core logic so much.

This should be submited to edk2-devel anyway, maintaing a modded "fork" is more work than getting the changes upstream once.

[MICKHAEL]

 

It took a 2-3 days but I figured out that...

 

In Clover

If I have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:

1) Boot into Windows and Linux without any problems

2) I CAN NOT boot into High Sierra in any way, my screen is corrupt

High Sierra with CsmVideoDxe-64.efi in my drivers64UEFI folder

 

If I DONT't have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:

1) Boot into High Sierra without any problems

2) I CAN NOT boot into Windows and Linux

Linux and Windows without CsmVideoDxe-64.efi in my drivers64UEFI folder (just a cursor)

 

MY QUESTION IS...

Is there a solution to this problem? Can I keep CsmVideoDxe-64.efi in my folder so, Windows and Linux will boot and disable it from High Sierra? Should I just take CsmVideoDxe-64.efi out but... How do I get into Windows and Linux through Clover then? Is there another way to fix this problem???

 

I have:

Intel Core i7 7700K 4.2 Ghz

Gigabyte GA-Z270MX/Gaming 5

Samsung CF791 Series 34-Inch Curved Widescreen Monitor (C34F791)  3440x1440 4K

Samsung U28E590D 28-Inch UHD LED-Lit Monitor with Freesync support  3840x2160 4K

AMD Radeon Pro Vega Air-Cooled Frontier Edition 16 GB

ANY HELP WOULD BE GREAT! I attached my config.plist if that helps in any way for somebody.

 

I think you enabled CSM and installed Windows/Linux in MBR ... search how to convert GPT, then disable CSM

[nms]

The compiler do not warning variable size difference? BOOLEAN <-> UINTN

For gcc (linux) or VS (Windows). But they are not care about clang.

We may just wait while someone send a message to edk2 list with the claim and someone resolves it.

Was already filed as bug #410.

 

The solution -- hide bugs under rag )-;

[RehabMan]

Was already filed as bug #410.

Thanks for pointing this out.

It is the fix I have decided to implement in my Clover fork.

 

Because really there is no edk2 bug here, just an overly pedantic compiler.

[electrovalent]

electro i had a similar issue on my old desktop with HD4000 it wound up being i had CSM clover driver. i had to remove it.

 

 

 

clover 4213 does not build

With or without the driver doesn't make any change.

[bronxteck]

gm converted my GUID/HFS drive to APFS i had a working windows 10 dual boot on it which clover handled. now trying to boot windows after the conversion running clover 4220 i get

sanity_check_alloced_blocks:144: fs_alloc_count mismatch: fs root nodes 86624 extent 6427 omap 1140 snap_meta 1 udata: 9228711 fs_alloc_count 9550063 efi_container_free:512: leaked 112

does that make sense or look familiar to anyone out there?

[phi777]

Maybe this link about apfs reversing could be useful https://blog.cugu.eu/post/apfs/ ?

[Matgen84]

 

It took a 2-3 days but I figured out that...

 

In Clover

If I have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:

1) Boot into Windows and Linux without any problems

2) I CAN NOT boot into High Sierra in any way, my screen is corrupt

High Sierra with CsmVideoDxe-64.efi in my drivers64UEFI folder

 

If I DONT't have CsmVideoDxe-64.efi in my drivers64UEFI folder, I can use Clover to:

1) Boot into High Sierra without any problems

2) I CAN NOT boot into Windows and Linux

Linux and Windows without CsmVideoDxe-64.efi in my drivers64UEFI folder (just a cursor)

 

MY QUESTION IS...

Is there a solution to this problem? Can I keep CsmVideoDxe-64.efi in my folder so, Windows and Linux will boot and disable it from High Sierra? Should I just take CsmVideoDxe-64.efi out but... How do I get into Windows and Linux through Clover then? Is there another way to fix this problem???

 

I have:

Intel Core i7 7700K 4.2 Ghz

Gigabyte GA-Z270MX/Gaming 5

Samsung CF791 Series 34-Inch Curved Widescreen Monitor (C34F791)  3440x1440 4K

Samsung U28E590D 28-Inch UHD LED-Lit Monitor with Freesync support  3840x2160 4K

AMD Radeon Pro Vega Air-Cooled Frontier Edition 16 GB

ANY HELP WOULD BE GREAT! I attached my config.plist if that helps in any way for somebody.

 

If all OS is UEFI installed. Try to disabled CSM in Bios and test