ifcfg-rh: merge branch 'th/ifcfg-parse-route-file-cleanup'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/392
This commit is contained in:
Thomas Haller 2020-01-16 12:41:05 +01:00
commit 3925ff4833
7 changed files with 212 additions and 82 deletions

View file

@ -1362,15 +1362,27 @@ nmtst_file_get_contents (const char *filename)
return contents;
}
#define nmtst_file_set_contents(filename, content) \
#define nmtst_file_set_contents_size(filename, content, size) \
G_STMT_START { \
GError *_error = NULL; \
gboolean _success; \
const char *_content = (content); \
gssize _size = (size); \
\
_success = g_file_set_contents ((filename), (content), -1, &_error); \
g_assert (_content); \
\
if (_size < 0) { \
g_assert (_size == -1); \
_size = strlen (_content); \
} \
\
_success = g_file_set_contents ((filename), _content, _size, &_error); \
nmtst_assert_success (_success, _error); \
} G_STMT_END
#define nmtst_file_set_contents(filename, content) \
nmtst_file_set_contents_size (filename, content, -1)
/*****************************************************************************/
static inline void

View file

@ -1345,39 +1345,40 @@ read_one_ip4_route (shvarFile *ifcfg,
}
static gboolean
read_route_file (int addr_family,
const char *filename,
NMSettingIPConfig *s_ip,
GError **error)
read_route_file_parse (int addr_family,
const char *filename,
const char *contents,
gsize len,
NMSettingIPConfig *s_ip,
GError **error)
{
gs_free char *contents = NULL;
char *contents_rest = NULL;
const char *line;
gsize len = 0;
gsize line_num;
g_return_val_if_fail (filename, FALSE);
g_return_val_if_fail ( (addr_family == AF_INET && NM_IS_SETTING_IP4_CONFIG (s_ip))
|| (addr_family == AF_INET6 && NM_IS_SETTING_IP6_CONFIG (s_ip)), FALSE);
g_return_val_if_fail (!error || !*error, FALSE);
nm_assert (filename);
nm_assert (addr_family == nm_setting_ip_config_get_addr_family (s_ip));
nm_assert (!error || !*error);
if ( !g_file_get_contents (filename, &contents, &len, NULL)
|| !len) {
if (len <= 0)
return TRUE; /* missing/empty = success */
}
line_num = 0;
for (line = strtok_r (contents, "\n", &contents_rest);
line;
line = strtok_r (NULL, "\n", &contents_rest)) {
while (TRUE) {
nm_auto_unref_ip_route NMIPRoute *route = NULL;
gs_free_error GError *local = NULL;
const char *line = contents;
char *eol;
int e;
eol = strchr (contents, '\n');
if (eol) {
eol[0] = '\0';
contents = &eol[1];
}
line_num++;
if (parse_route_line_is_comment (line))
continue;
goto next;
e = parse_route_line (line, addr_family, NULL, &route, &local);
@ -1389,14 +1390,38 @@ read_route_file (int addr_family,
* entire connection. */
PARSE_WARNING ("ignoring invalid route at \"%s\" (%s:%lu): %s", line, filename, (long unsigned) line_num, local->message);
}
continue;
goto next;
}
if (!nm_setting_ip_config_add_route (s_ip, route))
PARSE_WARNING ("duplicate IPv%c route", addr_family == AF_INET ? '4' : '6');
}
return TRUE;
next:
if (!eol)
return TRUE;
/* restore original content. */
eol[0] = '\n';
}
}
static gboolean
read_route_file (int addr_family,
const char *filename,
NMSettingIPConfig *s_ip,
GError **error)
{
gs_free char *contents = NULL;
gsize len;
nm_assert (filename);
nm_assert (addr_family == nm_setting_ip_config_get_addr_family (s_ip));
nm_assert (!error || !*error);
if (!g_file_get_contents (filename, &contents, &len, NULL))
return TRUE; /* missing/empty = success */
return read_route_file_parse (addr_family, filename, contents, len, s_ip, error);
}
static void
@ -1595,7 +1620,6 @@ make_ip4_setting (shvarFile *ifcfg,
int i;
guint32 a;
gboolean has_key;
shvarFile *route_ifcfg;
gboolean never_default;
gint64 i64;
int priority;
@ -1832,32 +1856,34 @@ make_ip4_setting (shvarFile *ifcfg,
/* Static routes - route-<name> file */
route_path = utils_get_route_path (svFileGetName (ifcfg));
if (!routes_read) {
/* NOP */
} else if (utils_has_route_file_new_syntax (route_path)) {
/* Parse route file in new syntax */
route_ifcfg = utils_get_route_ifcfg (svFileGetName (ifcfg), FALSE);
if (route_ifcfg) {
for (i = 0;; i++) {
NMIPRoute *route = NULL;
if (routes_read) {
gs_free char *contents = NULL;
gsize len;
if (!read_one_ip4_route (route_ifcfg, i, &route, error)) {
svCloseFile (route_ifcfg);
if (!g_file_get_contents (route_path, &contents, &len, NULL))
len = 0;
if (utils_has_route_file_new_syntax_content (contents, len)) {
nm_auto_shvar_file_close shvarFile *route_ifcfg = NULL;
/* Parse route file in new syntax */
route_ifcfg = svFile_new (route_path, -1, contents);
for (i = 0;; i++) {
nm_auto_unref_ip_route NMIPRoute *route = NULL;
if (!read_one_ip4_route (route_ifcfg, i, &route, error))
return NULL;
}
if (!route)
break;
if (!nm_setting_ip_config_add_route (s_ip4, route))
PARSE_WARNING ("duplicate IP4 route");
nm_ip_route_unref (route);
}
svCloseFile (route_ifcfg);
} else {
if (!read_route_file_parse (AF_INET, route_path, contents, len, s_ip4, error))
return NULL;
}
} else {
if (!read_route_file (AF_INET, route_path, s_ip4, error))
return NULL;
}
/* Legacy value NM used for a while but is incorrect (rh #459370) */

View file

@ -260,27 +260,60 @@ utils_get_route_ifcfg (const char *parent, gboolean should_create)
gboolean
utils_has_route_file_new_syntax (const char *filename)
{
char *contents = NULL;
gsize len = 0;
gboolean ret = FALSE;
const char *pattern = "^[[:space:]]*ADDRESS[0-9]+=";
gs_free char *contents_data = NULL;
gsize len;
g_return_val_if_fail (filename != NULL, TRUE);
if (!g_file_get_contents (filename, &contents, &len, NULL))
if (!g_file_get_contents (filename, &contents_data, &len, NULL))
return TRUE;
if (len <= 0) {
ret = TRUE;
goto gone;
return utils_has_route_file_new_syntax_content (contents_data, len);
}
gboolean
utils_has_route_file_new_syntax_content (const char *contents,
gsize len)
{
if (len <= 0)
return TRUE;
while (TRUE) {
const char *line = contents;
char *eol;
gboolean found = FALSE;
/* matches regex "^[[:space:]]*ADDRESS[0-9]+=" */
eol = (char *) strchr (contents, '\n');
if (eol) {
eol[0] = '\0';
contents = &eol[1];
}
line = nm_str_skip_leading_spaces (line);
if (NM_STR_HAS_PREFIX (line, "ADDRESS")) {
line += NM_STRLEN ("ADDRESS");
if (g_ascii_isdigit (line[0])) {
while (g_ascii_isdigit ((++line)[0])) {
/* pass */
}
if (line[0] == '=')
found = TRUE;
}
}
if (eol) {
/* restore the line ending. We don't want to mangle the content from
* POV of the caller. */
eol[0] = '\n';
}
if (found)
return TRUE;
if (!eol)
return FALSE;
}
if (g_regex_match_simple (pattern, contents, G_REGEX_MULTILINE, 0))
ret = TRUE;
gone:
g_free (contents);
return ret;
}
gboolean

View file

@ -76,6 +76,8 @@ shvarFile *utils_get_keys_ifcfg (const char *parent, gboolean should_create);
shvarFile *utils_get_route_ifcfg (const char *parent, gboolean should_create);
gboolean utils_has_route_file_new_syntax (const char *filename);
gboolean utils_has_route_file_new_syntax_content (const char *contents,
gsize len);
gboolean utils_has_complex_routes (const char *filename, int addr_family);
gboolean utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg);

View file

@ -71,6 +71,10 @@ struct _shvarFile {
/*****************************************************************************/
static void _line_link_parse (shvarFile *s, const char *value, gsize len);
/*****************************************************************************/
#define ASSERT_key_is_well_known(key) \
nm_assert ( ({ \
const char *_key = (key); \
@ -629,16 +633,33 @@ out_error:
/*****************************************************************************/
static shvarFile *
svFile_new (const char *name)
shvarFile *
svFile_new (const char *name,
int fd,
const char *content)
{
shvarFile *s;
const char *p;
const char *q;
nm_assert (name);
nm_assert (fd >= -1);
s = g_slice_new (shvarFile);
*s = (shvarFile) {
.fileName = g_strdup (name),
.fd = fd,
.lst_head = C_LIST_INIT (s->lst_head),
.lst_idx = g_hash_table_new (nm_pstr_hash, nm_pstr_equal),
};
if (content) {
for (p = content; (q = strchr (p, '\n')) != NULL; p = q + 1)
_line_link_parse (s, p, q - p);
if (p[0])
_line_link_parse (s, p, strlen (p));
}
s = g_slice_new0 (shvarFile);
s->fd = -1;
s->fileName = g_strdup (name);
c_list_init (&s->lst_head);
s->lst_idx = g_hash_table_new (nm_pstr_hash, nm_pstr_equal);
return s;
}
@ -839,11 +860,9 @@ do_link:
static shvarFile *
svOpenFileInternal (const char *name, gboolean create, GError **error)
{
shvarFile *s;
gboolean closefd = FALSE;
int errsv = 0;
gs_free char *arena = NULL;
const char *p, *q;
gs_free char *content = NULL;
gs_free_error GError *local = NULL;
nm_auto_close int fd = -1;
@ -860,7 +879,7 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
if (fd < 0) {
if (create)
return svFile_new (name);
return svFile_new (name, -1, NULL);
g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv),
"Could not read file '%s': %s",
@ -872,12 +891,12 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
closefd,
10 * 1024 * 1024,
NM_UTILS_FILE_GET_CONTENTS_FLAG_NONE,
&arena,
&content,
NULL,
NULL,
&local)) {
if (create)
return svFile_new (name);
return svFile_new (name, -1, NULL);
g_set_error (error, G_FILE_ERROR,
local->domain == G_FILE_ERROR ? local->code : G_FILE_ERROR_FAILED,
@ -886,21 +905,14 @@ svOpenFileInternal (const char *name, gboolean create, GError **error)
return NULL;
}
s = svFile_new (name);
for (p = arena; (q = strchr (p, '\n')) != NULL; p = q + 1)
_line_link_parse (s, p, q - p);
if (p[0])
_line_link_parse (s, p, strlen (p));
/* closefd is set if we opened the file read-only, so go ahead and
* close it, because we can't write to it anyway */
if (!closefd) {
nm_assert (fd > 0);
s->fd = nm_steal_fd (&fd);
}
return s;
nm_assert (closefd || fd >= 0);
return svFile_new (name,
!closefd
? nm_steal_fd (&fd)
: -1,
content);
}
/* Open the file <name>, return shvarFile on success, NULL on failure */

View file

@ -24,12 +24,20 @@ const char *svFileGetName (const shvarFile *s);
void _nmtst_svFileSetName (shvarFile *s, const char *fileName);
void _nmtst_svFileSetModified (shvarFile *s);
/*****************************************************************************/
shvarFile *svFile_new (const char *name,
int fd,
const char *content);
/* Create the file <name>, return a shvarFile (never fails) */
shvarFile *svCreateFile (const char *name);
/* Open the file <name>, return shvarFile on success, NULL on failure */
shvarFile *svOpenFile (const char *name, GError **error);
/*****************************************************************************/
const char *svFindFirstNumberedKey (shvarFile *s, const char *key_prefix);
/* Get the value associated with the key, and leave the current pointer

View file

@ -10305,6 +10305,42 @@ test_well_known_keys (void)
/*****************************************************************************/
static void
_do_utils_has_route_file_new_syntax_size (gboolean has_new_syntax,
const char *content,
gssize content_len)
{
nmtst_auto_unlinkfile char *testfile = g_strdup (TEST_SCRATCH_DIR"/utils-has-route-file-new-syntax-test.txt");
gboolean val;
nmtst_file_set_contents_size (testfile, content, content_len);
val = utils_has_route_file_new_syntax (testfile);
g_assert_cmpint (val, ==, has_new_syntax);
}
#define _do_utils_has_route_file_new_syntax(has_new_syntax, content) \
_do_utils_has_route_file_new_syntax_size (has_new_syntax, (content), NM_STRLEN (content))
static void
test_utils_has_route_file_new_syntax (void)
{
_do_utils_has_route_file_new_syntax (TRUE, "");
_do_utils_has_route_file_new_syntax (FALSE, "\0");
_do_utils_has_route_file_new_syntax (FALSE, "\n");
_do_utils_has_route_file_new_syntax (FALSE, "ADDRESS=bogus");
_do_utils_has_route_file_new_syntax (FALSE, "ADDRESS=bogus\0");
_do_utils_has_route_file_new_syntax (TRUE, "ADDRESS1=b\0ogus\0");
_do_utils_has_route_file_new_syntax (TRUE, "ADDRESS1=bogus\0");
_do_utils_has_route_file_new_syntax (TRUE, "\n\n\tADDRESS1=bogus\0");
_do_utils_has_route_file_new_syntax (FALSE, "\n\n\tADDRESS=bogus\n");
_do_utils_has_route_file_new_syntax (TRUE, "\n\n\tADDRESS=bogus\n ADDRESS000=\n");
_do_utils_has_route_file_new_syntax (FALSE, "\n\n\tROUTE1=bogus\n ADDRES=\n");
_do_utils_has_route_file_new_syntax (FALSE, "\n\n\tADDRESS=bogus\n ADDRESS\000000=\n");
}
/*****************************************************************************/
#define TPATH "/settings/plugins/ifcfg-rh/"
#define TEST_IFCFG_WIFI_OPEN_SSID_LONG_QUOTED TEST_IFCFG_DIR"/ifcfg-test-wifi-open-ssid-long-quoted"
@ -10602,6 +10638,7 @@ int main (int argc, char **argv)
g_test_add_func (TPATH "tc/read", test_tc_read);
g_test_add_func (TPATH "tc/write", test_tc_write);
g_test_add_func (TPATH "utils/test_well_known_keys", test_well_known_keys);
g_test_add_func (TPATH "utils/test_utils_has_route_file_new_syntax", test_utils_has_route_file_new_syntax);
return g_test_run ();
}