> On Mon, Sep 08, 2025 at 11:41:55AM +0200, Jan Beulich wrote: > > On 08.09.2025 11:35, Gerald Elder-Vass wrote: > > >>> + size == 1 && data == 0) ) > > >> > > >> ... any reason it's literal 1 here? > > > > > > The size variable is also used as output from GetVariable and we should > > > verify that the size of the returned data is as expected, it is simply one > > > byte so probably not worth defining any macros to make it clearer > > > > I don't understand this reply. Why would the initializer of the variable > > use one thing (sizeof()) and the checking of the variable another (literal > > 1)? Even consistently using 1 would already be better imo; consistently > > using sizeof() is what I think would be best. > > 'size' as input value is the allocated size of the data parameter, so > makes sense to be sizeof(data). IOW, 'size' as the input value comes > from the size of the 'data' variable, while the output value check comes > from UEFI spec. While the size of the 'data' variable should match the >spec, IMO changing its type (to a wider one) should not break the >behavior here.
The UEFI spec defines the "SecureBoot" global variable as an 8-bit unsigned integer, in the event the size of the data used for output was not large enough to contain the output then it would return an EFI_BUFFER_TOO_SMALL status (which the function would then interpret as "play it safe and assume enabled"). The "SecureBoot" variable is defined: https://uefi.org/specs/UEFI/2.11/03_Boot_Manager.html#globally-defined-variables So I believe we are correct in using uint8_t here *Gerald Elder-Vass* Senior Software Engineer XenServer Cambridge, UK On Mon, Sep 8, 2025 at 10:53 AM Marek Marczykowski-Górecki < [email protected]> wrote: > On Mon, Sep 08, 2025 at 11:41:55AM +0200, Jan Beulich wrote: > > On 08.09.2025 11:35, Gerald Elder-Vass wrote: > > >>> + size == 1 && data == 0) ) > > >> > > >> ... any reason it's literal 1 here? > > > > > > The size variable is also used as output from GetVariable and we should > > > verify that the size of the returned data is as expected, it is simply > one > > > byte so probably not worth defining any macros to make it clearer > > > > I don't understand this reply. Why would the initializer of the variable > > use one thing (sizeof()) and the checking of the variable another > (literal > > 1)? Even consistently using 1 would already be better imo; consistently > > using sizeof() is what I think would be best. > > 'size' as input value is the allocated size of the data parameter, so > makes sense to be sizeof(data). IOW, 'size' as the input value comes > from the size of the 'data' variable, while the output value check comes > from UEFI spec. While the size of the 'data' variable should match the > spec, IMO changing its type (to a wider one) should not break the > behavior here. > > -- > Best Regards, > Marek Marczykowski-Górecki > Invisible Things Lab >
