Re: [patch NetworkManager] ifcfg-rh: reader: allow device to do not have ipv4 setting.

You should add a test case to test-ifcfg-rh for this, to make sure we
don't break it later.

Other than that, just nitpicks:

Drop the word "do" in the commit summary line.

+static gchar *get_numbered_tag (gchar *tag_name, gint32 which)

Should be split onto two lines. Also, we don't generally use "gchar"
(just use "char"), and there's no particular reason to use gint32 rather
than int here. (I see that there are some existing gint32s in that
file... I have no idea why.)

+static gboolean is_any_ip4_address_defined (shvarFile *ifcfg)

likewise, split onto two lines.

Hm... so this function does the same thing as the code it replaced, but
I'm not sure why the old code did it that way... if there's just a
PREFIX, but no IPADDR, then that shouldn't count as "an IP4 address is
defined". Maybe dcbw will have some insight on this when he gets back.

+     if (!value || bootproto_none) {
              /* If there is no BOOTPROTO, no IPADDR, no PREFIX, no NETMASK, but

The comment there should be updated to mention BOOTPROTO=none

-- Dan

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]