On 2021-11-25 5:05 a.m., Mark Kettenis wrote:
From: Ted Bullock <[email protected]>
On 2021-11-25 3:55 a.m., Otto Moerbeek wrote:
+ parent = OF_parent(handle);
I think the OF_parent call can go inside the !strcmp(buf, "block") block.
I worried that the following re-assignment of the handle would cause
problems, so I chose an order of operations and placed the parent call
before this re-assignment. The reason for my concern is based on not
knowing the proper distinction between OF_finddevice and OF_open in the
boot prom itself (they are both kind of opaque to me and just send bits
for interpretation into the boot prom).
if ((handle = OF_open(fname)) == -1) {
DNPRINTF(BOOT_D_OFDEV, "devopen: open of %s failed\n", fname);
return ENXIO;
}
It's possible that there is no meaningful distinction between when
handle is re-assigned midway through the function, in which case you are
correct it could absolutely be moved later in the function call.
Notably there is quite a bit of variable re-use and re-purposing in the
devopen function so I chose the place I thought would be safest.
I think you have a point. Might make sense to have a separate
variables here. Probably best to keep using handle for the OF_open()
result, and use "node" or "dhandle" for the OF_finddevice() result.
More to the point I just went to test since I was now awake and thinking
about the problem some more and moving the call to OF_parent past the
reassignment with OF_open does indeed break booting. I presume that the
handle return value is contextually entirely different between
OF_finddevice and OF_open.
In fact, I will need to go through the recovery process again to install
a working boot block :P So further testing will need to be when it's not
5:22 AM and people won't yell at me.
Honestly there are so many ifdefs, gotos and variable re tasking in
these functions to handle building distinct boot binaries that the
entire file is worryingly fragile.
Anyway one thing at a time, I don't think its appropriate to be renaming
variables in this patch but I am willing to do that work on subsequent
ones if people want to see that done.
--
Ted Bullock <[email protected]>
