On Thu, Jul 29, 2021 at 09:30:49AM -0400, Stefan Berger wrote: > On 7/28/21 9:25 AM, Daniel Kiper wrote: > > On Tue, Jul 20, 2021 at 05:14:49PM -0400, Stefan Berger wrote: > > > > > +#define IEEE1275_CELL_TRUE ((grub_ieee1275_cell_t) -1) > > This smells like global constant. Does not it? If yes could you define it > > in a global header and use it? Maybe even replace existing comparisons > > in the IEEE1275 code with IEEE1275_CELL_TRUE. But probably then > > s/IEEE1275_CELL_TRUE/GRUB_IEEE1275_CELL_TRUE/... > > I wasn't sure and also had found local #defines in another .c file. > > #define IEEE1275_PHANDLE_INVALID ((grub_ieee1275_cell_t) -1) > #define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_cell_t) 0) > #define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1) > > https://git.savannah.gnu.org/cgit/grub.git/tree/grub-core/kern/ieee1275/ieee1275.c#n24 > > I haven't seen the usage of -1 as TRUE in other grub files. So I could move > it to a global header file assuming this is commonly used on this platform. > I have only seen usage of -1 as TRUE in the SLOF firmware in this file here: > > https://github.com/aik/SLOF/blob/master/lib/libnvram/libnvram.code#L66 > > but then of course also here: > https://github.com/aik/SLOF/blob/master/lib/libtpm/tcgbios.c#L965
I think we should use IEEE1275_CELL_INVALID in your code. IMO the TRUE sounds a bit confusing in this context. So, I would move all these three constants to a global IEEE1275 header and add "GRUB_" prefix. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
