All metadata.c files are very similar, with only trivial changes. Unify them
into a single common file, with minor special-casing where needed.
This has been compile-tested only so far, with tinderbox.
Details
Details
Build and run where able
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I think this is a great idea. Don't see any obvious problems with the implementation.
| stand/loader.mk | ||
|---|---|---|
| 20 ↗ | (On Diff #38194) | Shouldn't this only be built for ubldr on arm? |
| stand/common/metadata.c | ||
|---|---|---|
| 111–147 ↗ | (On Diff #38194) | This needs to be centralized. We have too many copies of it. While this is a good start, more needs to be done. Not a change request, per se, just an observation. This does help a lot, however. |
| 157–165 ↗ | (On Diff #38194) | Why not md_setbootargs? And require that the md code provide one, or a common one that does the else? We need to do something similar with EFI and parsing ConnIn / ConnOut and ConnErr... |
| 276–284 ↗ | (On Diff #38194) | So vm_offset_t can be different between 32-bit and 64-bit builds. Do we need to worry about that? |
| stand/loader.mk | ||
| 20 ↗ | (On Diff #38194) | We really need to start just building everything and putting it into a library. This twisty maze of ifdefs is insane. |
| stand/common/metadata.c | ||
|---|---|---|
| 111–147 ↗ | (On Diff #38194) | A lot more work needs to be done as a whole. Let's save that for the next step of cleanup. |
| 157–165 ↗ | (On Diff #38194) | That can be done. I wasn't too excited about the \#ifdef here anyway, but figured if we dump sparc64 soonish, this will go away anyway. Then we don't even need to bother with a md_setbootargs(). |
| 276–284 ↗ | (On Diff #38194) | Yes, but it's only significant on powerpc, where vm_offset_t is 32-bit, but this can load 64-bit kernels. MIPS has this code as well, but the 32-bit case is never used. If we \#ifdef out the 'unused' md_load*() function, this should be compiled down to virtually the same code as before on all platforms, unless the compiler isn't smart enough to elide the unused kern64 argument. |
| stand/loader.mk | ||
| 20 ↗ | (On Diff #38194) | I agree. Step 42 in cleanup? |
| 20 ↗ | (On Diff #38194) | I could move this, until said library is created, into arm/uboot/Makefile. Going into the efiloader it's just unused code (does our linker prune executables? It'd be nice if it did). |
| stand/loader.mk | ||
|---|---|---|
| 20 ↗ | (On Diff #38194) | I just took a look. The problem isn't that it isn't used for EFI. The problem EFI has it's own copy of these routines that are similar yet different in efi/loader/bootinfo.c (and bizarrely in efi/loader/arch/i386/bootinfo.c, but that one looks to be a bit-rotted version of the former). Perhaps you could look into the differences to see if they are real or not? |
| stand/loader.mk | ||
|---|---|---|
| 20 ↗ | (On Diff #38194) | A quick look at the file indicates the only differences are bi_load_efi_data(), and one small bit in bi_load(), adding MODINFOMD_FW_HANDLE. Beyond that, it looks identical to md_load(). Kill it with fire. |
| stand/loader.mk | ||
|---|---|---|
| 20 ↗ | (On Diff #38194) | Fire in this commit, or a future one? |
| stand/loader.mk | ||
|---|---|---|
| 20 ↗ | (On Diff #38194) | Future commit. I don't want to risk the vast majority of users just yet. I'd rather an x86 user do it, too. |
Comment Actions
Update per feedback. Passes make universe.
One thing left to do, but will be done after this change, will be to add the
swizzling from r328536. As that was contentious, I won't add it immediately,
but will make it a separate change.