Wed, Aug 21, 2013 at 06:08:19PM CEST, [email protected] wrote:
>You should add a test case to test-ifcfg-rh for this, to make sure we
>don't break it later.
Will try.
>
>
>Other than that, just nitpicks:
>
>Drop the word "do" in the commit summary line.
>
>> +static gchar *get_numbered_tag (gchar *tag_name, gint32 which)
>
Sure I will split this.
>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.)
With this I always try to be conform with the rest of the code.
Since tag_name is put as a parameter to g_strdup, which has gchar * in
its prototype, I'm using gchar.
That is similar with "gint32 which". read_full_ip4_address() has it as
gint32 as its parameter so I use it as well.
The questiion really is why all over the NM code this is mixed up.
Either g- of non-g- variant should be used (I beliveve that g-)
>
>> +static gboolean is_any_ip4_address_defined (shvarFile *ifcfg)
>
>likewise, split onto two lines.
Allright.
>
>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.
I'm just following the original code. If you want to change the
behaviour, I suggest to do it in separate patch.
>
>> + 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
Allright.
>
>-- Dan
_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list
