loader.efi environment related cleanups

[tsoome]

Summary

Since we have dedicated libefi/env.c file for variable support, the following
changes are done:

Simple cstyle changes in env.c
Moved efi variable related commands from loader/main.c to libefi/env.c
Did create function to set "efi-version" environment variable in env.c.

This function does serve two purposes: for first a  small clean up of the
loader main(), and for second, it does replace the otherwise unused
efi_variable_support hack.

A bit of cleanup of ficl backend functions. The TEST_MAIN has no meaning,
and removed few memory leaks.

The forth code is updated to use "efi-version" variable, instead of ficl
environment check.

Test Plan

Build and boot test. I have not yet done buildworld.

[imp]

OK. The code motion is fine. However the UTF conversion is wrong. It mostly works for ASCII / English, but fails for more complicated things. libefivar has the right routines, and it would be good to find a way for libstand to share that with libefivar and use the libstand routines. That can be done in a followup commit though.

sys/boot/efi/libefi/env.c

109 ↗

(On Diff #23951)

Does %S due strictly wide characters, or does it do UTF-16 encoded strings? That was one of my cleanup mental notes.

122–125 ↗

(On Diff #23951)

I'm not so sure about this heuristic.... I wrote it, but I'm not longer sure it's a reasonable thing. Comments?

sys/boot/efi/libefi/wchar.c

2 ↗

(On Diff #23951)

Yea, you didn't write these routines, I did. However, they are wrong. I forgot to go back and do them correctly based on getting libefivar working.

39 ↗

(On Diff #23951)

This is wrong. It was wrong before you started though. See what libefivar does, since that's right. Likewise for cpy8to16 and cpy16to8. It's only mostly right :(

[tsoome]

sys/boot/efi/libefi/env.c

109 ↗

(On Diff #23951)

%S is just sending CHAR16 string to console as is, assuming the SimpleTextOutput protocol is dealing with it.

122–125 ↗

(On Diff #23951)

We actually need to check the names of the variables and interpret based on the description in UEFI specification. For example, the some of them are to be converted to devicepath strings etc.

sys/boot/efi/libefi/wchar.c

2 ↗

(On Diff #23951)

Oh yes, I was just moving the functions and did not think too much:D can you please provide proper line for it, so I'll update before going for commit.

[tsoome]

ficlEfiSetenv() already does initialize the string variables

[tsoome]

libefi/Makefile needs to build efi.c even without BOOT_FORTH

[tsoome]

fixed the copyright line

[smh]

sys/boot/efi/libefi/env.c

156 ↗

(On Diff #24085)

Alignment issues or just reviews.?

176 ↗

(On Diff #24085)

unneeded warp?

209 ↗

(On Diff #24085)

missing \n

250 ↗

(On Diff #24085)

Can't this check be done earlier in a why that makes it clear how may args are really supported?

272 ↗

(On Diff #24085)

switch might make this clearer to read as it clearly associates the EFI_SUCCESS check below with this request.

274 ↗

(On Diff #24085)

Do we not have a realloc?

[tsoome]

use realloc(), add missing \n.

[tsoome]

sys/boot/efi/libefi/env.c

156 ↗

(On Diff #24085)

This is phabricator issue with dealing with tabs.

176 ↗

(On Diff #24085)

&uuid_status does not fit into 80 chars line.

250 ↗

(On Diff #24085)

I'm quite sure it can be, but I would leave it as is for now, as the main purpose for this change is to move the code from efi/loader/main.c. Also there are some additional changes related to variables, so we have to return to this code anyhow, and then it is better time to review the logic here.

272 ↗

(On Diff #24085)

This re-allocation also means we need to go for next loop iteration to repeat the GetNextVariableName() with larger buffer, the switch() will complicate the usage of "continue" there.

274 ↗

(On Diff #24085)

we do indeed.

sys/boot/efi/libefi/wchar.c

39 ↗

(On Diff #23951)

Yep, agree, but I do not want to touch it in this update, leaving those for followups.

[tsoome]

realloc() need to use temporary variable for result check, or
we will get memory leak.