On Wed, Aug 29, 2018 at 04:51:46PM +0800, Michael Chang wrote: > Actually I also have a similar patch posted one month ago for the very same > problem. > > https://lists.gnu.org/archive/html/grub-devel/2018-07/msg00103.html > > But as long as either way will do, I don't mind which one is going to be > reviewed and eventually merged. :)
I need to send the one with the fixed message. The problem I see with your approach is that some fields might still be uninitialized and maybe used later, hence I went with the full-on zeroes everywhere approach. > > On Thu, Aug 23, 2018 at 02:12:56PM +0200, Julian Andres Klode wrote: > > Code later on checks if variables inside the struct are > > 0 to see if they have been set, like if there were addresses > > in the bootpath. > > > > The variables were not initialized however, so the check > > might suceed with uninitialized data, and a new interface > > with random addresses has been added. This caused a weird > > bug in Ubuntu, because when booting from network, we now > > had two interfaces with the same name, and net_default_mac > > pointed to the random one. > > > > Bug-Ubuntu: https://bugs.launchpad.net/bugs/1785859 > > Signed-off-by: Julian Andres Klode <julian.kl...@canonical.com> > > --- > > grub-core/net/drivers/ieee1275/ofnet.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/grub-core/net/drivers/ieee1275/ofnet.c > > b/grub-core/net/drivers/ieee1275/ofnet.c > > index 002446be1..00abc64bb 100644 > > --- a/grub-core/net/drivers/ieee1275/ofnet.c > > +++ b/grub-core/net/drivers/ieee1275/ofnet.c > > @@ -153,8 +153,8 @@ grub_ieee1275_parse_bootpath (const char *devpath, char > > *bootpath, > > char *comma_char = 0; > > char *equal_char = 0; > > grub_size_t field_counter = 0; > > - grub_net_network_level_address_t client_addr, gateway_addr, subnet_mask; > > - grub_net_link_level_address_t hw_addr; > > + grub_net_network_level_address_t client_addr = {}, gateway_addr = {}, > > subnet_mask = {}; > > + grub_net_link_level_address_t hw_addr = {}; > > Well. I personally don't like the idea to use empty initializer list as some > compiler would reject it since it is not defined in ISO C standard to be > useful > to initialize "rest" of the structure to zero, if only part of the initialzer > is provided. The gcc accepts it but is an extenstion syntax anyway. grub requires gcc(-style) compilers, though, and makes use of GNU C throughout the code. > > I think better to have at least one initialzer on the list, like {0} or > similar, which is standard compliant and also more readable. Then you have a problem if the structure layout changes, unless you use a named initializer. -- debian developer - deb.li/jak | jak-linux.org - free software dev ubuntu core developer i speak de, en _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
