> Date: Thu, 25 Nov 2021 04:28:55 -0700
> Content-Language: en-US
> Cc: Theo de Raadt <dera...@openbsd.org>,
> Mark Kettenis <mark.kette...@xs4all.nl>, bugs@openbsd.org
> From: Ted Bullock <tbull...@comlore.com>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> 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.
Cheers,
Mark
