diff options
author | Dan Winship <danw@gnome.org> | 2014-03-18 13:13:12 -0400 |
---|---|---|
committer | Dan Winship <danw@gnome.org> | 2014-04-09 10:50:56 -0400 |
commit | e43283a288ffe9f00520400a94eef3593ad8d782 (patch) | |
tree | 38ca10daf6b2a71711689e27412320784292c247 | |
parent | 454311c9ecca2fc42029dd5f7c4e65b8bcb193db (diff) | |
download | NetworkManager-e43283a288ffe9f00520400a94eef3593ad8d782.tar.gz |
ifcfg-rh: return proper error messages from svOpenFile() and svWriteFile()
-rw-r--r-- | src/settings/plugins/ifcfg-rh/plugin.c | 6 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/reader.c | 19 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/shvar.c | 57 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/shvar.h | 10 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 49 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/utils.c | 2 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/writer.c | 27 |
7 files changed, 96 insertions, 74 deletions
diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c index 5fda59da09..0bf16758b5 100644 --- a/src/settings/plugins/ifcfg-rh/plugin.c +++ b/src/settings/plugins/ifcfg-rh/plugin.c @@ -646,7 +646,7 @@ plugin_get_hostname (SCPluginIfcfg *plugin) return hostname; } - network = svOpenFile (SC_NETWORK_FILE); + network = svOpenFile (SC_NETWORK_FILE, NULL); if (!network) { PLUGIN_WARN (IFCFG_PLUGIN_NAME, "Could not get hostname: failed to read " SC_NETWORK_FILE); return NULL; @@ -708,10 +708,10 @@ plugin_set_hostname (SCPluginIfcfg *plugin, const char *hostname) g_free (hostname_eol); /* Remove "HOSTNAME" from SC_NETWORK_FILE, if present */ - network = svOpenFile (SC_NETWORK_FILE); + network = svOpenFile (SC_NETWORK_FILE, NULL); if (network) { svSetValue (network, "HOSTNAME", NULL, FALSE); - svWriteFile (network, 0644); + svWriteFile (network, 0644, NULL); svCloseFile (network); } diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index 7a0a41649e..3b0379c200 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -682,7 +682,7 @@ read_full_ip4_address (shvarFile *ifcfg, gboolean read_success; /* If no gateway in the ifcfg, try /etc/sysconfig/network instead */ - network_ifcfg = svOpenFile (network_file); + network_ifcfg = svOpenFile (network_file, NULL); if (network_ifcfg) { read_success = read_ip4_address (network_ifcfg, "GATEWAY", &tmp, error); svCloseFile (network_ifcfg); @@ -1066,7 +1066,7 @@ parse_full_ip6_address (shvarFile *ifcfg, } if (!value) { /* If no gateway in the ifcfg, try global /etc/sysconfig/network instead */ - network_ifcfg = svOpenFile (network_file); + network_ifcfg = svOpenFile (network_file, NULL); if (network_ifcfg) { value = svGetValue (network_ifcfg, "IPV6_DEFAULTGW", FALSE); svCloseFile (network_ifcfg); @@ -1281,7 +1281,7 @@ make_ip4_setting (shvarFile *ifcfg, never_default = !svTrueValue (ifcfg, "DEFROUTE", TRUE); /* Then check if GATEWAYDEV; it's global and overrides DEFROUTE */ - network_ifcfg = svOpenFile (network_file); + network_ifcfg = svOpenFile (network_file, NULL); if (network_ifcfg) { char *gatewaydev; @@ -1645,7 +1645,7 @@ make_ip6_setting (shvarFile *ifcfg, * they are global and override IPV6_DEFROUTE * When both are set, the device specified in IPV6_DEFAULTGW takes preference. */ - network_ifcfg = svOpenFile (network_file); + network_ifcfg = svOpenFile (network_file, NULL); if (network_ifcfg) { char *ipv6_defaultgw, *ipv6_defaultdev; char *default_dev = NULL; @@ -1680,7 +1680,7 @@ make_ip6_setting (shvarFile *ifcfg, str_value = svGetValue (ifcfg, "IPV6INIT", FALSE); ipv6init = svTrueValue (ifcfg, "IPV6INIT", FALSE); if (!str_value) { - network_ifcfg = svOpenFile (network_file); + network_ifcfg = svOpenFile (network_file, NULL); if (network_ifcfg) { ipv6init = svTrueValue (network_ifcfg, "IPV6INIT", FALSE); svCloseFile (network_ifcfg); @@ -4991,7 +4991,7 @@ uuid_from_file (const char *filename) if (!ifcfg_name) return NULL; - ifcfg = svOpenFile (filename); + ifcfg = svOpenFile (filename, NULL); if (!ifcfg) return NULL; @@ -5077,12 +5077,9 @@ connection_from_file (const char *filename, return NULL; } - parsed = svOpenFile (filename); - if (!parsed) { - g_set_error (error, IFCFG_PLUGIN_ERROR, 0, - "Couldn't parse file '%s'", filename); + parsed = svOpenFile (filename, error); + if (!parsed) return NULL; - } if (!svTrueValue (parsed, "NM_CONTROLLED", TRUE)) { g_assert (out_unhandled != NULL); diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c index 630e9b6fd1..4ba8a631f1 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.c +++ b/src/settings/plugins/ifcfg-rh/shvar.c @@ -39,10 +39,11 @@ * (actually, return a structure anyway) if it doesn't exist. */ static shvarFile * -svOpenFileInternal (const char *name, gboolean create) +svOpenFileInternal (const char *name, gboolean create, GError **error) { shvarFile *s = NULL; gboolean closefd = FALSE; + int errsv; s = g_slice_new0 (shvarFile); @@ -53,7 +54,9 @@ svOpenFileInternal (const char *name, gboolean create) if (!create || s->fd == -1) { /* try read-only */ s->fd = open (name, O_RDONLY); /* NOT O_CREAT */ - if (s->fd != -1) + if (s->fd == -1) + errsv = errno; + else closefd = TRUE; } s->fileName = g_strdup (name); @@ -63,8 +66,10 @@ svOpenFileInternal (const char *name, gboolean create) char *arena, *p, *q; ssize_t nread, total = 0; - if (fstat (s->fd, &buf) < 0) + if (fstat (s->fd, &buf) < 0) { + errsv = errno; goto bail; + } arena = g_malloc (buf.st_size + 1); arena[buf.st_size] = '\0'; @@ -72,8 +77,10 @@ svOpenFileInternal (const char *name, gboolean create) nread = read (s->fd, arena + total, buf.st_size - total); if (nread == -1 && errno == EINTR) continue; - if (nread <= 0) + if (nread <= 0) { + errsv = errno; goto bail; + } total += nread; } @@ -101,14 +108,18 @@ svOpenFileInternal (const char *name, gboolean create) close (s->fd); g_free (s->fileName); g_slice_free (shvarFile, s); + + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), + "Could not read file '%s': %s", + name, errsv ? strerror (errsv) : "Unknown error"); return NULL; } /* Open the file <name>, return shvarFile on success, NULL on failure */ shvarFile * -svOpenFile (const char *name) +svOpenFile (const char *name, GError **error) { - return svOpenFileInternal (name, FALSE); + return svOpenFileInternal (name, FALSE, error); } /* Create a new file structure, returning actual data if the file exists, @@ -117,7 +128,7 @@ svOpenFile (const char *name) shvarFile * svCreateFile (const char *name) { - return svOpenFileInternal (name, TRUE); + return svOpenFileInternal (name, TRUE, NULL); } /* remove escaped characters in place */ @@ -360,7 +371,7 @@ svSetValue (shvarFile *s, const char *key, const char *value, gboolean verbatim) * open() syscall. */ gboolean -svWriteFile (shvarFile *s, int mode) +svWriteFile (shvarFile *s, int mode, GError **error) { FILE *f; int tmpfd; @@ -368,14 +379,32 @@ svWriteFile (shvarFile *s, int mode) if (s->modified) { if (s->fd == -1) s->fd = open (s->fileName, O_WRONLY | O_CREAT, mode); - if (s->fd == -1) + if (s->fd == -1) { + int errsv = errno; + + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), + "Could not open file '%s' for writing: %s", + s->fileName, strerror (errsv)); return FALSE; - if (ftruncate (s->fd, 0) < 0) + } + if (ftruncate (s->fd, 0) < 0) { + int errsv = errno; + + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), + "Could not overwrite file '%s': %s", + s->fileName, strerror (errsv)); return FALSE; + } tmpfd = dup (s->fd); - if (tmpfd == -1) + if (tmpfd == -1) { + int errsv = errno; + + g_set_error (error, G_FILE_ERROR, g_file_error_from_errno (errsv), + "Internal error writing file '%s': %s", + s->fileName, strerror (errsv)); return FALSE; + } f = fdopen (tmpfd, "w"); fseek (f, 0, SEEK_SET); for (s->current = s->lineList; s->current; s->current = s->current->next) { @@ -390,10 +419,10 @@ svWriteFile (shvarFile *s, int mode) /* Close the file descriptor (if open) and free the shvarFile. */ -gboolean +void svCloseFile (shvarFile *s) { - g_return_val_if_fail (s != NULL, FALSE); + g_return_if_fail (s != NULL); if (s->fd != -1) close (s->fd); @@ -401,6 +430,4 @@ svCloseFile (shvarFile *s) g_free (s->fileName); g_list_free_full (s->lineList, g_free); /* implicitly frees s->current */ g_slice_free (shvarFile, s); - - return TRUE; } diff --git a/src/settings/plugins/ifcfg-rh/shvar.h b/src/settings/plugins/ifcfg-rh/shvar.h index d28bc6d7d5..4cbf1a31a7 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.h +++ b/src/settings/plugins/ifcfg-rh/shvar.h @@ -49,7 +49,7 @@ struct _shvarFile { shvarFile *svCreateFile (const char *name); /* Open the file <name>, return shvarFile on success, NULL on failure */ -shvarFile *svOpenFile (const char *name); +shvarFile *svOpenFile (const char *name, GError **error); /* Get the value associated with the key, and leave the current pointer * pointing at the line containing the value. The char* returned MUST @@ -77,12 +77,10 @@ void svSetValue (shvarFile *s, const char *key, const char *value, gboolean verb * re-writing an existing file, and is passed unchanged to the * open() syscall. */ -gboolean svWriteFile (shvarFile *s, int mode); +gboolean svWriteFile (shvarFile *s, int mode, GError **error); -/* Close the file descriptor (if open) and free the shvarFile. - * Returns FALSE on error and TRUE on success. - */ -gboolean svCloseFile (shvarFile *s); +/* Close the file descriptor (if open) and free the shvarFile. */ +void svCloseFile (shvarFile *s); /* Return a new escaped string */ char *svEscape (const char *s); diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 4d774fa967..082d6fd585 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -6106,12 +6106,13 @@ test_write_wifi_hidden (void) TEST_SCRATCH_DIR "/network-scripts/", &testfile, &error); - f = svOpenFile (testfile); - g_assert (f); - g_assert_no_error (error); g_assert (success); + f = svOpenFile (testfile, &error); + g_assert_no_error (error); + g_assert (f); + /* re-read the file to check that what key was written. */ val = svGetValue (f, "SSID_HIDDEN", FALSE); g_assert (val); @@ -7337,11 +7338,16 @@ test_write_wired_static_ip6_only_gw (gconstpointer user_data) NULL, NULL, &error, &ignore_error); + g_assert_no_error (error); + g_assert (reread); + g_assert (nm_connection_verify (reread, &error)); + g_assert (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); { /* re-read the file to check that what key was written. */ - shvarFile *ifcfg = svOpenFile (testfile); + shvarFile *ifcfg = svOpenFile (testfile, &error); + g_assert_no_error (error); g_assert (ifcfg); written_ifcfg_gateway = svGetValue (ifcfg, "IPV6_DEFAULTGW", FALSE); svCloseFile (ifcfg); @@ -7349,11 +7355,6 @@ test_write_wired_static_ip6_only_gw (gconstpointer user_data) unlink (testfile); - g_assert_no_error (error); - g_assert (reread); - g_assert (nm_connection_verify (reread, &error)); - g_assert (nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); - /* access the gateway from the loaded connection. */ s_ip6 = nm_connection_get_setting_ip6_config (reread); g_assert (s_ip6 && nm_setting_ip6_config_get_num_addresses (s_ip6)==1); @@ -8380,11 +8381,12 @@ test_write_wifi_open (void) &route6file, &error, &ignore_error); + g_assert_no_error (error); /* Now make sure that the ESSID item isn't double-quoted (rh #606518) */ - ifcfg = svOpenFile (testfile); - ASSERT (ifcfg != NULL, - "wifi-open-write-reread", "failed to load %s as shvarfile", testfile); + ifcfg = svOpenFile (testfile, &error); + g_assert_no_error (error); + g_assert (ifcfg != NULL); tmp = svGetValue (ifcfg, "ESSID", TRUE); ASSERT (tmp != NULL, @@ -10869,7 +10871,8 @@ test_write_wifi_dynamic_wep_leap (void) * did not get written. Check first that the auth alg is not set to "LEAP" * and next that the only IEEE 802.1x EAP method is "LEAP". */ - ifcfg = svOpenFile (testfile); + ifcfg = svOpenFile (testfile, &error); + g_assert_no_error (error); g_assert (ifcfg); tmp = svGetValue (ifcfg, "SECURITYMODE", FALSE); g_assert_cmpstr (tmp, ==, NULL); @@ -11487,7 +11490,8 @@ test_write_wired_ctc_dhcp (void) g_assert (testfile != NULL); /* Ensure the CTCPROT item gets written out as it's own option */ - ifcfg = svOpenFile (testfile); + ifcfg = svOpenFile (testfile, &error); + g_assert_no_error (error); g_assert (ifcfg); tmp = svGetValue (ifcfg, "CTCPROT", TRUE); @@ -13864,9 +13868,10 @@ test_write_fcoe_mode (gconstpointer user_data) g_assert (testfile); { - shvarFile *ifcfg = svOpenFile (testfile); + shvarFile *ifcfg = svOpenFile (testfile, &error); char *written_mode; + g_assert_no_error (error); g_assert (ifcfg); written_mode = svGetValue (ifcfg, "DCB_APP_FCOE_MODE", FALSE); svCloseFile (ifcfg); @@ -13975,12 +13980,13 @@ test_write_team_master (void) TEST_SCRATCH_DIR "/network-scripts/", &testfile, &error); - f = svOpenFile (testfile); - g_assert (f); - g_assert_no_error (error); g_assert (success); + f = svOpenFile (testfile, &error); + g_assert_no_error (error); + g_assert (f); + /* re-read the file to check that what key was written. */ val = svGetValue (f, "DEVICETYPE", FALSE); g_assert (val); @@ -14092,12 +14098,13 @@ test_write_team_port (void) TEST_SCRATCH_DIR "/network-scripts/", &testfile, &error); - f = svOpenFile (testfile); - g_assert (f); - g_assert_no_error (error); g_assert (success); + f = svOpenFile (testfile, &error); + g_assert_no_error (error); + g_assert (f); + /* re-read the file to check that what key was written. */ val = svGetValue (f, "TYPE", FALSE); g_assert (!val); diff --git a/src/settings/plugins/ifcfg-rh/utils.c b/src/settings/plugins/ifcfg-rh/utils.c index 07efa5cc53..11b8a52e8d 100644 --- a/src/settings/plugins/ifcfg-rh/utils.c +++ b/src/settings/plugins/ifcfg-rh/utils.c @@ -296,7 +296,7 @@ utils_get_extra_ifcfg (const char *parent, const char *tag, gboolean should_crea ifcfg = svCreateFile (path); if (!ifcfg) - ifcfg = svOpenFile (path); + ifcfg = svOpenFile (path, NULL); g_free (path); return ifcfg; diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index 9c7b3037a3..1f0e6a9dda 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -97,6 +97,7 @@ set_secret (shvarFile *ifcfg, gboolean verbatim) { shvarFile *keyfile; + GError *error = NULL; /* Clear the secret from the ifcfg and the associated "keys" file */ svSetValue (ifcfg, key, NULL, FALSE); @@ -118,9 +119,9 @@ set_secret (shvarFile *ifcfg, if (flags == NM_SETTING_SECRET_FLAG_NONE) svSetValue (keyfile, key, value, verbatim); - if (!svWriteFile (keyfile, 0600)) { - PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: could not update key file '%s'", - keyfile->fileName); + if (!svWriteFile (keyfile, 0600, &error)) { + PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: %s", error->message); + g_clear_error (&error); svCloseFile (keyfile); goto error; } @@ -2118,9 +2119,7 @@ write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) g_free (gw_key); g_free (metric_key); } - if (!svWriteFile (routefile, 0644)) { - g_set_error (error, IFCFG_PLUGIN_ERROR, 0, - "Could not update route file '%s'", routefile->fileName); + if (!svWriteFile (routefile, 0644, error)) { svCloseFile (routefile); goto out; } @@ -2529,7 +2528,10 @@ write_connection (NMConnection *connection, if (filename) { /* For existing connections, 'filename' should be full path to ifcfg file */ - ifcfg = svOpenFile (filename); + ifcfg = svOpenFile (filename, error); + if (!ifcfg) + return FALSE; + ifcfg_name = g_strdup (filename); } else { char *escaped; @@ -2565,12 +2567,6 @@ write_connection (NMConnection *connection, ifcfg = svCreateFile (ifcfg_name); } - if (!ifcfg) { - g_set_error (error, IFCFG_PLUGIN_ERROR, 0, - "Failed to open/create ifcfg file '%s'", ifcfg_name); - goto out; - } - type = nm_setting_connection_get_connection_type (s_con); if (!type) { g_set_error (error, IFCFG_PLUGIN_ERROR, 0, @@ -2641,11 +2637,8 @@ write_connection (NMConnection *connection, write_connection_setting (s_con, ifcfg); - if (!svWriteFile (ifcfg, 0644)) { - g_set_error (error, IFCFG_PLUGIN_ERROR, 0, - "Can't write connection '%s'", ifcfg->fileName); + if (!svWriteFile (ifcfg, 0644, error)) goto out; - } /* Only return the filename if this was a newly written ifcfg */ if (out_filename && !filename) |