ifcfg/trivial: rename svGetValueString() to svGetValueStr_cp()

We have

 - svGetValue()
    - returns the original string
    - avoids copying the string unless necessary

 - svGetValueStr_cp() (formerly svGetValueString())
    - returns the original string, unless it is empty ""
    - always clones the string

I think the behavior svGetValueStr*() of coercing "" to NULL is wrongly
used in most places. We should better handle "" like any other value,
not treat it as unset.

That would require another function svGetValue_cp(), which is like svGetValue()
but always copies the string. Rename svGetValueString() so that there is a place
for names like

  - svGetValue_cp()
  - svGetValueStr()

Also rename svSetValueString() to svSetValueStr().
This commit is contained in:
Thomas Haller 2017-02-21 12:07:14 +01:00
parent d37c77234f
commit c0c9b3dbcd
5 changed files with 393 additions and 393 deletions

File diff suppressed because it is too large Load diff

File diff suppressed because it is too large Load diff

View file

@ -917,7 +917,7 @@ svGetValue (shvarFile *s, const char *key, char **to_free)
* be freed by the caller.
*/
char *
svGetValueString (shvarFile *s, const char *key)
svGetValueStr_cp (shvarFile *s, const char *key)
{
char *to_free;
const char *value;
@ -991,8 +991,8 @@ svGetValueInt64 (shvarFile *s, const char *key, guint base, gint64 min, gint64 m
/*****************************************************************************/
/* Same as svSetValueString() but it preserves empty @value -- contrary to
* svSetValueString() for which "" effectively means to remove the value. */
/* Same as svSetValueStr() but it preserves empty @value -- contrary to
* svSetValueStr() for which "" effectively means to remove the value. */
void
svSetValue (shvarFile *s, const char *key, const char *value)
{
@ -1041,7 +1041,7 @@ svSetValue (shvarFile *s, const char *key, const char *value)
* to the bottom of the file.
*/
void
svSetValueString (shvarFile *s, const char *key, const char *value)
svSetValueStr (shvarFile *s, const char *key, const char *value)
{
svSetValue (s, key, value && value[0] ? value : NULL);
}

View file

@ -49,7 +49,7 @@ shvarFile *svOpenFile (const char *name, GError **error);
* be freed by the caller.
*/
const char *svGetValue (shvarFile *s, const char *key, char **to_free);
char *svGetValueString (shvarFile *s, const char *key);
char *svGetValueStr_cp (shvarFile *s, const char *key);
gint svParseBoolean (const char *value, gint def);
@ -66,7 +66,7 @@ gint64 svGetValueInt64 (shvarFile *s, const char *key, guint base, gint64 min, g
* the key=value pair after that line. Otherwise, prepend the pair
* to the top of the file.
*/
void svSetValueString (shvarFile *s, const char *key, const char *value);
void svSetValueStr (shvarFile *s, const char *key, const char *value);
void svSetValue (shvarFile *s, const char *key, const char *value);
void svSetValueBoolean (shvarFile *s, const char *key, gboolean value);
void svSetValueInt64 (shvarFile *s, const char *key, gint64 value);

View file

@ -81,7 +81,7 @@
shvarFile *const _f = (f); \
const char *const _key = (key); \
\
_val_string = svGetValueString (_f, _key); \
_val_string = svGetValueStr_cp (_f, _key); \
_val = svGetValue (_f, _key, &_to_free); \
g_assert_cmpstr (_val, ==, (expected_value)); \
g_assert ( (!_val_string && (!_val || !_val[0])) \
@ -3275,7 +3275,7 @@ test_write_wired_wake_on_lan (void)
&testfile);
f = _svOpenFile (testfile);
val = svGetValueString (f, "ETHTOOL_OPTS");
val = svGetValueStr_cp (f, "ETHTOOL_OPTS");
g_assert (val);
g_assert (strstr (val, "wol"));
g_assert (strstr (val, "sopass 00:00:00:11:22:33"));
@ -3310,7 +3310,7 @@ test_write_wired_auto_negotiate_off (void)
&testfile);
f = _svOpenFile (testfile);
val = svGetValueString (f, "ETHTOOL_OPTS");
val = svGetValueStr_cp (f, "ETHTOOL_OPTS");
g_assert (val);
g_assert (strstr (val, "autoneg off"));
g_assert (strstr (val, "speed 10"));
@ -3344,7 +3344,7 @@ test_write_wired_auto_negotiate_on (void)
&testfile);
f = _svOpenFile (testfile);
val = svGetValueString (f, "ETHTOOL_OPTS");
val = svGetValueStr_cp (f, "ETHTOOL_OPTS");
g_assert (val);
g_assert (strstr (val, "autoneg on"));
g_assert (!strstr (val, "speed"));
@ -4097,7 +4097,7 @@ test_write_wired_static_ip6_only_gw (gconstpointer user_data)
nmtst_assert_connection_equals (connection, TRUE, reread, FALSE);
ifcfg = _svOpenFile (testfile);
written_ifcfg_gateway = svGetValueString (ifcfg, "IPV6_DEFAULTGW");
written_ifcfg_gateway = svGetValueStr_cp (ifcfg, "IPV6_DEFAULTGW");
svCloseFile (ifcfg);
/* access the gateway from the loaded connection. */
@ -4577,15 +4577,15 @@ test_write_wired_aliases (void)
/* Create some pre-existing alias files, to make sure they get overwritten / deleted. */
ifcfg = svCreateFile (TEST_SCRATCH_ALIAS_BASE ":2");
svSetValueString (ifcfg, "DEVICE", "alias0:2");
svSetValueString (ifcfg, "IPADDR", "192.168.1.2");
svSetValueStr (ifcfg, "DEVICE", "alias0:2");
svSetValueStr (ifcfg, "IPADDR", "192.168.1.2");
svWriteFile (ifcfg, 0644, NULL);
svCloseFile (ifcfg);
g_assert (g_file_test (TEST_SCRATCH_ALIAS_BASE ":2", G_FILE_TEST_EXISTS));
ifcfg = svCreateFile (TEST_SCRATCH_ALIAS_BASE ":5");
svSetValueString (ifcfg, "DEVICE", "alias0:5");
svSetValueString (ifcfg, "IPADDR", "192.168.1.5");
svSetValueStr (ifcfg, "DEVICE", "alias0:5");
svSetValueStr (ifcfg, "IPADDR", "192.168.1.5");
svWriteFile (ifcfg, 0644, NULL);
svCloseFile (ifcfg);
g_assert (g_file_test (TEST_SCRATCH_ALIAS_BASE ":5", G_FILE_TEST_EXISTS));