Hi, this series does a couple of code reorganizations/refactoring in code used by tools: the windows path handling code, and the two types of file editing (using editor, and using perl expression). There's still a code duplication between the two variants of file editing, but it is just within a single source, and can be easily solved now (planning as next step). Pino Toscano (13): edit: move windows path code to common file fish, edit: specifies whether mount Windows as readonly fish, edit: move the exit-on-case-sensitive-error behaviour to virt-edit cat: use the common Windows path handling code fish: isolate file editing (w/ editor) code in own file fish: edit: improve the editor execution fish: edit: bring backup extension to file editing w/ editor fish: edit: return 1 for unchanged file fish: edit: bring the fast-time-edit protection fish: edit: add perl file editing edit: switch to common editing functions fish: edit: add verbose parameter customize: use the common perl file editing code builder/Makefile.am | 2 + cat/Makefile.am | 4 +- cat/cat.c | 112 +-------------- customize/Makefile.am | 5 +- customize/perl_edit-c.c | 55 ++++++++ customize/perl_edit.ml | 62 +------- edit/Makefile.am | 6 +- edit/edit.c | 368 +++--------------------------------------------- fish/Makefile.am | 2 + fish/edit.c | 125 +--------------- fish/file-edit.c | 325 ++++++++++++++++++++++++++++++++++++++++++ fish/file-edit.h | 47 +++++++ fish/windows.c | 135 ++++++++++++++++++ fish/windows.h | 45 ++++++ po/POTFILES | 3 + sysprep/Makefile.am | 2 + v2v/Makefile.am | 2 + 17 files changed, 663 insertions(+), 637 deletions(-) create mode 100644 customize/perl_edit-c.c create mode 100644 fish/file-edit.c create mode 100644 fish/file-edit.h create mode 100644 fish/windows.c create mode 100644 fish/windows.h -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 01/13] edit: move windows path code to common file
Move the code handling Windows paths from virt-edit to a common file, so that can be shared by various tools. Mostly code motion, with a minimum touch (the additional guestfs_h* parameter in mount_drive_letter) to make it build and work. --- edit/Makefile.am | 4 +- edit/edit.c | 104 +----------------------------------------- fish/windows.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ fish/windows.h | 44 ++++++++++++++++++ po/POTFILES | 1 + 5 files changed, 184 insertions(+), 104 deletions(-) create mode 100644 fish/windows.c create mode 100644 fish/windows.h diff --git a/edit/Makefile.am b/edit/Makefile.am index 1d51095..cb19bec 100644 --- a/edit/Makefile.am +++ b/edit/Makefile.am @@ -33,7 +33,9 @@ SHARED_SOURCE_FILES = \ ../fish/options.h \ ../fish/options.c \ ../fish/uri.h \ - ../fish/uri.c + ../fish/uri.c \ + ../fish/windows.h \ + ../fish/windows.c virt_edit_SOURCES = \ $(SHARED_SOURCE_FILES) \ diff --git a/edit/edit.c b/edit/edit.c index 733c911..d4ce747 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -37,6 +37,7 @@ #include "guestfs.h" #include "options.h" +#include "windows.h" /* Currently open libguestfs handle. */ guestfs_h *g; @@ -56,8 +57,6 @@ static void edit_files (int argc, char *argv[]); static void edit (const char *filename, const char *root); static char *edit_interactively (const char *tmpfile); static char *edit_non_interactively (const char *tmpfile); -static int is_windows (guestfs_h *g, const char *root); -static char *windows_path (guestfs_h *g, const char *root, const char *filename); static char *generate_random_name (const char *filename); static char *generate_backup_name (const char *filename); @@ -527,107 +526,6 @@ edit_non_interactively (const char *tmpfile) return ret; /* caller will free */ } -static int -is_windows (guestfs_h *g, const char *root) -{ - int w; - CLEANUP_FREE char *type = guestfs_inspect_get_type (g, root); - if (!type) - return 0; - - w = STREQ (type, "windows"); - return w; -} - -static void mount_drive_letter (char drive_letter, const char *root); - -static char * -windows_path (guestfs_h *g, const char *root, const char *path) -{ - char *ret; - size_t i; - - /* If there is a drive letter, rewrite the path. */ - if (c_isalpha (path[0]) && path[1] == ':') { - char drive_letter = c_tolower (path[0]); - /* This returns the newly allocated string. */ - mount_drive_letter (drive_letter, root); - ret = strdup (path + 2); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - else if (!*path) { - ret = strdup ("/"); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - else { - ret = strdup (path); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - - /* Blindly convert any backslashes into forward slashes. Is this good? */ - for (i = 0; i < strlen (ret); ++i) - if (ret[i] == '\\') - ret[i] = '/'; - - char *t = guestfs_case_sensitive_path (g, ret); - free (ret); - ret = t; - if (ret == NULL) - exit (EXIT_FAILURE); - - return ret; -} - -static void -mount_drive_letter (char drive_letter, const char *root) -{ - char *device; - size_t i; - - /* Resolve the drive letter using the drive mappings table. */ - CLEANUP_FREE_STRING_LIST char **drives - guestfs_inspect_get_drive_mappings (g, root); - if (drives == NULL || drives[0] == NULL) { - fprintf (stderr, _("%s: to use Windows drive letters, this must be a Windows guest\n"), - program_name); - exit (EXIT_FAILURE); - } - - device = NULL; - for (i = 0; drives[i] != NULL; i += 2) { - if (c_tolower (drives[i][0]) == drive_letter && drives[i][1] == '\0') { - device = drives[i+1]; - break; - } - } - - if (device == NULL) { - fprintf (stderr, _("%s: drive '%c:' not found.\n"), - program_name, drive_letter); - exit (EXIT_FAILURE); - } - - /* Unmount current disk and remount device. */ - if (guestfs_umount_all (g) == -1) - exit (EXIT_FAILURE); - - if (guestfs_mount (g, device, "/") == -1) - exit (EXIT_FAILURE); - - /* Don't need to free (device) because that string was in the - * drives array. - */ -} - static char random_char (void) { diff --git a/fish/windows.c b/fish/windows.c new file mode 100644 index 0000000..ddf2af1 --- /dev/null +++ b/fish/windows.c @@ -0,0 +1,135 @@ +/* libguestfs - shared Windows path for tools + * Copyright (C) 2009-2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include "windows.h" + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> +#include <errno.h> +#include <locale.h> +#include <langinfo.h> +#include <libintl.h> + +#include "guestfs-internal-frontend.h" + +#include "c-ctype.h" + +static void mount_drive_letter (guestfs_h *g, char drive_letter, + const char *root); + +int +is_windows (guestfs_h *g, const char *root) +{ + int w; + CLEANUP_FREE char *type = guestfs_inspect_get_type (g, root); + if (!type) + return 0; + + w = STREQ (type, "windows"); + return w; +} + +char * +windows_path (guestfs_h *g, const char *root, const char *path) +{ + char *ret; + size_t i; + + /* If there is a drive letter, rewrite the path. */ + if (c_isalpha (path[0]) && path[1] == ':') { + char drive_letter = c_tolower (path[0]); + /* This returns the newly allocated string. */ + mount_drive_letter (g, drive_letter, root); + ret = strdup (path + 2); + if (ret == NULL) { + perror ("strdup"); + exit (EXIT_FAILURE); + } + } + else if (!*path) { + ret = strdup ("/"); + if (ret == NULL) { + perror ("strdup"); + exit (EXIT_FAILURE); + } + } + else { + ret = strdup (path); + if (ret == NULL) { + perror ("strdup"); + exit (EXIT_FAILURE); + } + } + + /* Blindly convert any backslashes into forward slashes. Is this good? */ + for (i = 0; i < strlen (ret); ++i) + if (ret[i] == '\\') + ret[i] = '/'; + + char *t = guestfs_case_sensitive_path (g, ret); + free (ret); + ret = t; + if (ret == NULL) + exit (EXIT_FAILURE); + + return ret; +} + +static void +mount_drive_letter (guestfs_h *g, char drive_letter, const char *root) +{ + char *device; + size_t i; + + /* Resolve the drive letter using the drive mappings table. */ + CLEANUP_FREE_STRING_LIST char **drives + guestfs_inspect_get_drive_mappings (g, root); + if (drives == NULL || drives[0] == NULL) { + fprintf (stderr, _("%s: to use Windows drive letters, this must be a Windows guest\n"), + program_name); + exit (EXIT_FAILURE); + } + + device = NULL; + for (i = 0; drives[i] != NULL; i += 2) { + if (c_tolower (drives[i][0]) == drive_letter && drives[i][1] == '\0') { + device = drives[i+1]; + break; + } + } + + if (device == NULL) { + fprintf (stderr, _("%s: drive '%c:' not found.\n"), + program_name, drive_letter); + exit (EXIT_FAILURE); + } + + /* Unmount current disk and remount device. */ + if (guestfs_umount_all (g) == -1) + exit (EXIT_FAILURE); + + if (guestfs_mount (g, device, "/") == -1) + exit (EXIT_FAILURE); + + /* Don't need to free (device) because that string was in the + * drives array. + */ +} diff --git a/fish/windows.h b/fish/windows.h new file mode 100644 index 0000000..bb68634 --- /dev/null +++ b/fish/windows.h @@ -0,0 +1,44 @@ +/* libguestfs - shared Windows path for tools + * Copyright (C) 2009-2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef FISH_WINDOWS_H +#define FISH_WINDOWS_H + +#include <guestfs.h> + +/** + * Checks whether 'root' is a Windows installation. + * + * This relies on an already being done introspection. + */ +extern int is_windows (guestfs_h *g, const char *root); + +/** + * Resolves 'path' as possible Windows path according to 'root', + * giving a new path that can be used in libguestfs API calls. + * + * Notes: + * - 'root' must be a Windows installation + * - relies on an already being done introspection + * - will unmount all the existing mount points and mount the Windows root + * - will exit() on memory allocation failures, and if it is not possible + * to get the true path on case-insensitive filesystems + */ +extern char *windows_path (guestfs_h *g, const char *root, const char *path); + +#endif diff --git a/po/POTFILES b/po/POTFILES index 3d208d5..d40cf9b 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -160,6 +160,7 @@ fish/supported.c fish/tilde.c fish/time.c fish/uri.c +fish/windows.c format/format.c fuse/guestmount.c fuse/guestunmount.c -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 02/13] fish, edit: specifies whether mount Windows as readonly
--- edit/edit.c | 3 ++- fish/windows.c | 11 ++++++----- fish/windows.h | 4 +++- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/edit/edit.c b/edit/edit.c index d4ce747..cd91b5f 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -336,7 +336,8 @@ edit (const char *filename, const char *root) /* Windows? Special handling is required. */ if (root != NULL && is_windows (g, root)) - filename = filename_to_free = windows_path (g, root, filename); + filename = filename_to_free = windows_path (g, root, filename, + 0 /* not read only */); /* Download the file to a temporary. */ fd = mkstemp (tmpfile); diff --git a/fish/windows.c b/fish/windows.c index ddf2af1..06dcab3 100644 --- a/fish/windows.c +++ b/fish/windows.c @@ -33,7 +33,7 @@ #include "c-ctype.h" static void mount_drive_letter (guestfs_h *g, char drive_letter, - const char *root); + const char *root, int readonly); int is_windows (guestfs_h *g, const char *root) @@ -48,7 +48,7 @@ is_windows (guestfs_h *g, const char *root) } char * -windows_path (guestfs_h *g, const char *root, const char *path) +windows_path (guestfs_h *g, const char *root, const char *path, int readonly) { char *ret; size_t i; @@ -57,7 +57,7 @@ windows_path (guestfs_h *g, const char *root, const char *path) if (c_isalpha (path[0]) && path[1] == ':') { char drive_letter = c_tolower (path[0]); /* This returns the newly allocated string. */ - mount_drive_letter (g, drive_letter, root); + mount_drive_letter (g, drive_letter, root, readonly); ret = strdup (path + 2); if (ret == NULL) { perror ("strdup"); @@ -94,7 +94,8 @@ windows_path (guestfs_h *g, const char *root, const char *path) } static void -mount_drive_letter (guestfs_h *g, char drive_letter, const char *root) +mount_drive_letter (guestfs_h *g, char drive_letter, const char *root, + int readonly) { char *device; size_t i; @@ -126,7 +127,7 @@ mount_drive_letter (guestfs_h *g, char drive_letter, const char *root) if (guestfs_umount_all (g) == -1) exit (EXIT_FAILURE); - if (guestfs_mount (g, device, "/") == -1) + if ((readonly ? guestfs_mount_ro : guestfs_mount) (g, device, "/") == -1) exit (EXIT_FAILURE); /* Don't need to free (device) because that string was in the diff --git a/fish/windows.h b/fish/windows.h index bb68634..5543182 100644 --- a/fish/windows.h +++ b/fish/windows.h @@ -36,9 +36,11 @@ extern int is_windows (guestfs_h *g, const char *root); * - 'root' must be a Windows installation * - relies on an already being done introspection * - will unmount all the existing mount points and mount the Windows root + * (according to 'readonly') * - will exit() on memory allocation failures, and if it is not possible * to get the true path on case-insensitive filesystems */ -extern char *windows_path (guestfs_h *g, const char *root, const char *path); +extern char *windows_path (guestfs_h *g, const char *root, const char *path, + int readonly); #endif -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 03/13] fish, edit: move the exit-on-case-sensitive-error behaviour to virt-edit
Do not unconditionally exit if guestfs_case_sensitive_path, but let windows_path still return null. Make virt-edit then check for that, and eventually exit on its own. --- edit/edit.c | 5 ++++- fish/windows.c | 3 +-- fish/windows.h | 3 +-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/edit/edit.c b/edit/edit.c index cd91b5f..1f5ec7b 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -335,9 +335,12 @@ edit (const char *filename, const char *root) CLEANUP_FREE char *backupname = NULL; /* Windows? Special handling is required. */ - if (root != NULL && is_windows (g, root)) + if (root != NULL && is_windows (g, root)) { filename = filename_to_free = windows_path (g, root, filename, 0 /* not read only */); + if (filename == NULL) + exit (EXIT_FAILURE); + } /* Download the file to a temporary. */ fd = mkstemp (tmpfile); diff --git a/fish/windows.c b/fish/windows.c index 06dcab3..04ead2f 100644 --- a/fish/windows.c +++ b/fish/windows.c @@ -84,11 +84,10 @@ windows_path (guestfs_h *g, const char *root, const char *path, int readonly) if (ret[i] == '\\') ret[i] = '/'; + /* If this fails, we want to return NULL. */ char *t = guestfs_case_sensitive_path (g, ret); free (ret); ret = t; - if (ret == NULL) - exit (EXIT_FAILURE); return ret; } diff --git a/fish/windows.h b/fish/windows.h index 5543182..c543604 100644 --- a/fish/windows.h +++ b/fish/windows.h @@ -37,8 +37,7 @@ extern int is_windows (guestfs_h *g, const char *root); * - relies on an already being done introspection * - will unmount all the existing mount points and mount the Windows root * (according to 'readonly') - * - will exit() on memory allocation failures, and if it is not possible - * to get the true path on case-insensitive filesystems + * - will exit() on memory allocation failures */ extern char *windows_path (guestfs_h *g, const char *root, const char *path, int readonly); -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 04/13] cat: use the common Windows path handling code
--- cat/Makefile.am | 4 +- cat/cat.c | 112 ++------------------------------------------------------ 2 files changed, 6 insertions(+), 110 deletions(-) diff --git a/cat/Makefile.am b/cat/Makefile.am index 55710a3..14b8e81 100644 --- a/cat/Makefile.am +++ b/cat/Makefile.am @@ -42,7 +42,9 @@ SHARED_SOURCE_FILES = \ ../fish/options.h \ ../fish/options.c \ ../fish/uri.h \ - ../fish/uri.c + ../fish/uri.c \ + ../fish/windows.h \ + ../fish/windows.c virt_cat_SOURCES = \ $(SHARED_SOURCE_FILES) \ diff --git a/cat/cat.c b/cat/cat.c index def8280..a1d9f83 100644 --- a/cat/cat.c +++ b/cat/cat.c @@ -33,6 +33,7 @@ #include "guestfs.h" #include "options.h" +#include "windows.h" /* Currently open libguestfs handle. */ guestfs_h *g; @@ -46,8 +47,6 @@ const char *libvirt_uri = NULL; int inspector = 1; static int do_cat (int argc, char *argv[]); -static int is_windows (guestfs_h *g, const char *root); -static char *windows_path (guestfs_h *g, const char *root, const char *filename); static void __attribute__((noreturn)) usage (int status) @@ -285,7 +284,8 @@ do_cat (int argc, char *argv[]) const char *filename = argv[i]; if (windows) { - filename = filename_to_free = windows_path (g, root, filename); + filename = filename_to_free = windows_path (g, root, filename, + 1 /* readonly */); if (filename == NULL) { errors++; continue; @@ -298,109 +298,3 @@ do_cat (int argc, char *argv[]) return errors == 0 ? 0 : -1; } - -static int -is_windows (guestfs_h *g, const char *root) -{ - char *type; - int w; - - type = guestfs_inspect_get_type (g, root); - if (!type) - return 0; - - w = STREQ (type, "windows"); - free (type); - return w; -} - -static void mount_drive_letter_ro (char drive_letter, const char *root); - -static char * -windows_path (guestfs_h *g, const char *root, const char *path) -{ - char *ret; - size_t i; - - /* If there is a drive letter, rewrite the path. */ - if (c_isalpha (path[0]) && path[1] == ':') { - char drive_letter = c_tolower (path[0]); - /* This returns the newly allocated string. */ - mount_drive_letter_ro (drive_letter, root); - ret = strdup (path + 2); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - else if (!*path) { - ret = strdup ("/"); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - else { - ret = strdup (path); - if (ret == NULL) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - } - - /* Blindly convert any backslashes into forward slashes. Is this good? */ - for (i = 0; i < strlen (ret); ++i) - if (ret[i] == '\\') - ret[i] = '/'; - - /* If this fails, we want to return NULL. */ - char *t = guestfs_case_sensitive_path (g, ret); - free (ret); - ret = t; - - return ret; -} - -static void -mount_drive_letter_ro (char drive_letter, const char *root) -{ - char **drives; - char *device; - size_t i; - - /* Resolve the drive letter using the drive mappings table. */ - drives = guestfs_inspect_get_drive_mappings (g, root); - if (drives == NULL || drives[0] == NULL) { - fprintf (stderr, _("%s: to use Windows drive letters, this must be a Windows guest\n"), - program_name); - exit (EXIT_FAILURE); - } - - device = NULL; - for (i = 0; drives[i] != NULL; i += 2) { - if (c_tolower (drives[i][0]) == drive_letter && drives[i][1] == '\0') { - device = drives[i+1]; - break; - } - } - - if (device == NULL) { - fprintf (stderr, _("%s: drive '%c:' not found.\n"), - program_name, drive_letter); - exit (EXIT_FAILURE); - } - - /* Unmount current disk and remount device. */ - if (guestfs_umount_all (g) == -1) - exit (EXIT_FAILURE); - - if (guestfs_mount_ro (g, device, "/") == -1) - exit (EXIT_FAILURE); - - for (i = 0; drives[i] != NULL; ++i) - free (drives[i]); - free (drives); - /* Don't need to free (device) because that string was in the - * drives array. - */ -} -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 05/13] fish: isolate file editing (w/ editor) code in own file
Move the implementation of file editing using editor to an own file, so that can be shared by different tools. Mostly code motion. --- fish/Makefile.am | 2 + fish/edit.c | 125 ++----------------------------------------- fish/file-edit.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ fish/file-edit.h | 32 +++++++++++ po/POTFILES | 1 + 5 files changed, 198 insertions(+), 120 deletions(-) create mode 100644 fish/file-edit.c create mode 100644 fish/file-edit.h diff --git a/fish/Makefile.am b/fish/Makefile.am index c02f703..d28a94b 100644 --- a/fish/Makefile.am +++ b/fish/Makefile.am @@ -87,6 +87,8 @@ guestfish_SOURCES = \ echo.c \ edit.c \ events.c \ + file-edit.c \ + file-edit.h \ fish.c \ fish.h \ glob.c \ diff --git a/fish/edit.c b/fish/edit.c index bd02f4b..5b851e7 100644 --- a/fish/edit.c +++ b/fish/edit.c @@ -22,29 +22,20 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> -#include <fcntl.h> #include <inttypes.h> #include <libintl.h> -#include <sys/types.h> -#include <sys/stat.h> -#include <assert.h> #include "fish.h" - -static char *generate_random_name (const char *filename); +#include "file-edit.h" /* guestfish edit command, suggested by Ján Ondrej, implemented by RWMJ */ int run_edit (const char *cmd, size_t argc, char *argv[]) { - CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); - CLEANUP_UNLINK_FREE char *filename = NULL; - char buf[256]; const char *editor; - CLEANUP_FREE char *remotefilename = NULL, *newname = NULL; - struct stat oldstat, newstat; - int r, fd; + CLEANUP_FREE char *remotefilename = NULL; + int r; if (argc != 1) { fprintf (stderr, _("use '%s filename' to edit a file\n"), cmd); @@ -67,113 +58,7 @@ run_edit (const char *cmd, size_t argc, char *argv[]) if (remotefilename == NULL) return -1; - /* Download the file and write it to a temporary. */ - if (asprintf (&filename, "%s/guestfishXXXXXX", tmpdir) == -1) { - perror ("asprintf"); - return -1; - } - - fd = mkstemp (filename); - if (fd == -1) { - perror ("mkstemp"); - return -1; - } - - snprintf (buf, sizeof buf, "/dev/fd/%d", fd); - - if (guestfs_download (g, remotefilename, buf) == -1) { - close (fd); - return -1; - } - - if (close (fd) == -1) { - perror (filename); - return -1; - } - - /* Get the old stat. */ - if (stat (filename, &oldstat) == -1) { - perror (filename); - return -1; - } - - /* Edit it. */ - /* XXX Safe? */ - snprintf (buf, sizeof buf, "%s %s", editor, filename); - - r = system (buf); - if (r != 0) { - perror (buf); - return -1; - } - - /* Get the new stat. */ - if (stat (filename, &newstat) == -1) { - perror (filename); - return -1; - } - - /* Changed? */ - if (oldstat.st_ctime == newstat.st_ctime && - oldstat.st_size == newstat.st_size) - return 0; - - /* Upload to a new file in the same directory, so if it fails we - * don't end up with a partially written file. Give the new file - * a completely random name so we have only a tiny chance of - * overwriting some existing file. - */ - newname = generate_random_name (remotefilename); - if (!newname) - return -1; - - /* Write new content. */ - if (guestfs_upload (g, filename, newname) == -1) - return -1; - - /* Set the permissions, UID, GID and SELinux context of the new - * file to match the old file (RHBZ#788641). - */ - if (guestfs_copy_attributes (g, remotefilename, newname, - GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1) - return -1; - - if (guestfs_mv (g, newname, remotefilename) == -1) - return -1; - - return 0; -} - -static char -random_char (void) -{ - char c[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; - return c[random () % (sizeof c - 1)]; -} - -static char * -generate_random_name (const char *filename) -{ - char *ret, *p; - size_t i; - - ret = malloc (strlen (filename) + 16); - if (!ret) { - perror ("malloc"); - return NULL; - } - strcpy (ret, filename); - - p = strrchr (ret, '/'); - assert (p); - p++; - - /* Because of "+ 16" above, there should be enough space in the - * output buffer to write 8 random characters here. - */ - for (i = 0; i < 8; ++i) - *p++ = random_char (); - *p++ = '\0'; + r = edit_file_editor (g, remotefilename, editor); - return ret; /* caller will free */ + return r; } diff --git a/fish/file-edit.c b/fish/file-edit.c new file mode 100644 index 0000000..ec707a9 --- /dev/null +++ b/fish/file-edit.c @@ -0,0 +1,158 @@ +/* libguestfs - shared file editing + * Copyright (C) 2009-2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include "file-edit.h" + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> +#include <errno.h> +#include <unistd.h> +#include <locale.h> +#include <langinfo.h> +#include <libintl.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <assert.h> + +#include "guestfs-internal-frontend.h" + +static char *generate_random_name (const char *filename); + +int +edit_file_editor (guestfs_h *g, const char *filename, const char *editor) +{ + CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); + CLEANUP_UNLINK_FREE char *tmpfilename = NULL; + char buf[256]; + CLEANUP_FREE char *newname = NULL; + struct stat oldstat, newstat; + int r, fd; + + /* Download the file and write it to a temporary. */ + if (asprintf (&tmpfilename, "%s/libguestfsXXXXXX", tmpdir) == -1) { + perror ("asprintf"); + return -1; + } + + fd = mkstemp (tmpfilename); + if (fd == -1) { + perror ("mkstemp"); + return -1; + } + + snprintf (buf, sizeof buf, "/dev/fd/%d", fd); + + if (guestfs_download (g, filename, buf) == -1) { + close (fd); + return -1; + } + + if (close (fd) == -1) { + perror (tmpfilename); + return -1; + } + + /* Get the old stat. */ + if (stat (tmpfilename, &oldstat) == -1) { + perror (tmpfilename); + return -1; + } + + /* Edit it. */ + /* XXX Safe? */ + snprintf (buf, sizeof buf, "%s %s", editor, filename); + + r = system (buf); + if (r != 0) { + perror (buf); + return -1; + } + + /* Get the new stat. */ + if (stat (tmpfilename, &newstat) == -1) { + perror (tmpfilename); + return -1; + } + + /* Changed? */ + if (oldstat.st_ctime == newstat.st_ctime && + oldstat.st_size == newstat.st_size) + return 0; + + /* Upload to a new file in the same directory, so if it fails we + * don't end up with a partially written file. Give the new file + * a completely random name so we have only a tiny chance of + * overwriting some existing file. + */ + newname = generate_random_name (filename); + if (!newname) + return -1; + + /* Write new content. */ + if (guestfs_upload (g, tmpfilename, newname) == -1) + return -1; + + /* Set the permissions, UID, GID and SELinux context of the new + * file to match the old file (RHBZ#788641). + */ + if (guestfs_copy_attributes (g, filename, newname, + GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1) + return -1; + + if (guestfs_mv (g, newname, filename) == -1) + return -1; + + return 0; +} + +static char +random_char (void) +{ + const char c[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + return c[random () % (sizeof c - 1)]; +} + +static char * +generate_random_name (const char *filename) +{ + char *ret, *p; + size_t i; + + ret = malloc (strlen (filename) + 16); + if (!ret) { + perror ("malloc"); + return NULL; + } + strcpy (ret, filename); + + p = strrchr (ret, '/'); + assert (p); + p++; + + /* Because of "+ 16" above, there should be enough space in the + * output buffer to write 8 random characters here. + */ + for (i = 0; i < 8; ++i) + *p++ = random_char (); + *p++ = '\0'; + + return ret; /* caller will free */ +} diff --git a/fish/file-edit.h b/fish/file-edit.h new file mode 100644 index 0000000..eff1c7d --- /dev/null +++ b/fish/file-edit.h @@ -0,0 +1,32 @@ +/* libguestfs - shared file editing + * Copyright (C) 2009-2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef FISH_FILE_EDIT_H +#define FISH_FILE_EDIT_H + +#include <guestfs.h> + +/** + * Edit 'filename' using the specified 'editor' application. + * + * Returns -1 for failure, 0 otherwise. + */ +extern int edit_file_editor (guestfs_h *g, const char *filename, + const char *editor); + +#endif diff --git a/po/POTFILES b/po/POTFILES index d40cf9b..a31cc88 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -135,6 +135,7 @@ fish/echo.c fish/edit.c fish/event-names.c fish/events.c +fish/file-edit.c fish/fish.c fish/glob.c fish/help.c -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 06/13] fish: edit: improve the editor execution
Dynamically allocate the buffer for the command, and check also its exit status. Improvements taken from the current implementation in virt-edit. --- fish/file-edit.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/fish/file-edit.c b/fish/file-edit.c index ec707a9..408d20e 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -43,6 +43,7 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor) CLEANUP_UNLINK_FREE char *tmpfilename = NULL; char buf[256]; CLEANUP_FREE char *newname = NULL; + CLEANUP_FREE char *cmd = NULL; struct stat oldstat, newstat; int r, fd; @@ -77,12 +78,14 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor) } /* Edit it. */ - /* XXX Safe? */ - snprintf (buf, sizeof buf, "%s %s", editor, filename); + if (asprintf (&cmd, "%s %s", editor, tmpfilename) == -1) { + perror ("asprintf"); + return -1; + } - r = system (buf); - if (r != 0) { - perror (buf); + r = system (cmd); + if (r == -1 || WEXITSTATUS (r) != 0) { + perror (cmd); return -1; } -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 07/13] fish: edit: bring backup extension to file editing w/ editor
Bring the backup extension feature from the perl-like file mode implementation, although currently not making use of it. --- fish/edit.c | 2 +- fish/file-edit.c | 31 ++++++++++++++++++++++++++++++- fish/file-edit.h | 4 +++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/fish/edit.c b/fish/edit.c index 5b851e7..0170daf 100644 --- a/fish/edit.c +++ b/fish/edit.c @@ -58,7 +58,7 @@ run_edit (const char *cmd, size_t argc, char *argv[]) if (remotefilename == NULL) return -1; - r = edit_file_editor (g, remotefilename, editor); + r = edit_file_editor (g, remotefilename, editor, NULL); return r; } diff --git a/fish/file-edit.c b/fish/file-edit.c index 408d20e..8efa452 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -35,9 +35,12 @@ #include "guestfs-internal-frontend.h" static char *generate_random_name (const char *filename); +static char *generate_backup_name (const char *filename, + const char *backup_extension); int -edit_file_editor (guestfs_h *g, const char *filename, const char *editor) +edit_file_editor (guestfs_h *g, const char *filename, const char *editor, + const char *backup_extension) { CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); CLEANUP_UNLINK_FREE char *tmpfilename = NULL; @@ -120,6 +123,17 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor) GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1) return -1; + /* Backup or overwrite the file. */ + if (backup_extension) { + CLEANUP_FREE char *backupname = NULL; + + backupname = generate_backup_name (filename, backup_extension); + if (backupname == NULL) + return -1; + + if (guestfs_mv (g, filename, backupname) == -1) + return -1; + } if (guestfs_mv (g, newname, filename) == -1) return -1; @@ -159,3 +173,18 @@ generate_random_name (const char *filename) return ret; /* caller will free */ } + +static char * +generate_backup_name (const char *filename, const char *backup_extension) +{ + char *ret; + + assert (backup_extension != NULL); + + if (asprintf (&ret, "%s%s", filename, backup_extension) == -1) { + perror ("asprintf"); + return NULL; + } + + return ret; /* caller will free */ +} diff --git a/fish/file-edit.h b/fish/file-edit.h index eff1c7d..6622b3a 100644 --- a/fish/file-edit.h +++ b/fish/file-edit.h @@ -23,10 +23,12 @@ /** * Edit 'filename' using the specified 'editor' application. + * If 'backup_extension' is not null, then a copy of 'filename' is saved + * with 'backup_extension' appended to its file name. * * Returns -1 for failure, 0 otherwise. */ extern int edit_file_editor (guestfs_h *g, const char *filename, - const char *editor); + const char *editor, const char *backup_extension); #endif -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 08/13] fish: edit: return 1 for unchanged file
Change the return value of edit_file_editor to 1, in case the editor did not change the (temporary) file. --- fish/edit.c | 2 +- fish/file-edit.c | 2 +- fish/file-edit.h | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/fish/edit.c b/fish/edit.c index 0170daf..f43ef3b 100644 --- a/fish/edit.c +++ b/fish/edit.c @@ -60,5 +60,5 @@ run_edit (const char *cmd, size_t argc, char *argv[]) r = edit_file_editor (g, remotefilename, editor, NULL); - return r; + return r == -1 ? -1 : 0; } diff --git a/fish/file-edit.c b/fish/file-edit.c index 8efa452..3a9dd2d 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -101,7 +101,7 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, /* Changed? */ if (oldstat.st_ctime == newstat.st_ctime && oldstat.st_size == newstat.st_size) - return 0; + return 1; /* Upload to a new file in the same directory, so if it fails we * don't end up with a partially written file. Give the new file diff --git a/fish/file-edit.h b/fish/file-edit.h index 6622b3a..1d401c6 100644 --- a/fish/file-edit.h +++ b/fish/file-edit.h @@ -26,7 +26,8 @@ * If 'backup_extension' is not null, then a copy of 'filename' is saved * with 'backup_extension' appended to its file name. * - * Returns -1 for failure, 0 otherwise. + * Returns -1 for failure, 0 on success, 1 if the editor did not change + * the file (e.g. the user closed the editor without saving). */ extern int edit_file_editor (guestfs_h *g, const char *filename, const char *editor, const char *backup_extension); -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 09/13] fish: edit: bring the fast-time-edit protection
Bring from virt-edit the small hack of putting the ctime of the temporary file few seconds back to notice file changes also on fast editing. --- fish/file-edit.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/fish/file-edit.c b/fish/file-edit.c index 3a9dd2d..e2298eb 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -31,6 +31,7 @@ #include <sys/types.h> #include <sys/stat.h> #include <assert.h> +#include <utime.h> #include "guestfs-internal-frontend.h" @@ -49,6 +50,7 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, CLEANUP_FREE char *cmd = NULL; struct stat oldstat, newstat; int r, fd; + struct utimbuf times; /* Download the file and write it to a temporary. */ if (asprintf (&tmpfilename, "%s/libguestfsXXXXXX", tmpdir) == -1) { @@ -74,6 +76,23 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, return -1; } + /* Set the time back a few seconds on the original file. This is so + * that if the user is very fast at editing, or if EDITOR is an + * automatic editor, then the edit might happen within the 1 second + * granularity of mtime, and we would think the file hasn't changed. + */ + if (stat (tmpfilename, &oldstat) == -1) { + perror (tmpfilename); + return -1; + } + + times.actime = oldstat.st_atime - 5; + times.modtime = oldstat.st_mtime - 5; + if (utime (tmpfilename, ×) == -1) { + perror ("utimes"); + return -1; + } + /* Get the old stat. */ if (stat (tmpfilename, &oldstat) == -1) { perror (tmpfilename); -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 10/13] fish: edit: add perl file editing
Add the perl file editing, mostly based in the virt-edit implementation. This introduces a mild code duplication with edit_file_editor; will deal with it in a later commit. --- fish/file-edit.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ fish/file-edit.h | 11 ++++++ 2 files changed, 121 insertions(+) diff --git a/fish/file-edit.c b/fish/file-edit.c index e2298eb..fa68626 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -159,6 +159,116 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, return 0; } +int +edit_file_perl (guestfs_h *g, const char *filename, const char *perl_expr, + const char *backup_extension) +{ + CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); + CLEANUP_UNLINK_FREE char *tmpfilename = NULL; + char buf[256]; + CLEANUP_FREE char *newname = NULL; + CLEANUP_FREE char *cmd = NULL; + CLEANUP_FREE char *outfile = NULL; + int r, fd; + + /* Download the file and write it to a temporary. */ + if (asprintf (&tmpfilename, "%s/libguestfsXXXXXX", tmpdir) == -1) { + perror ("asprintf"); + return -1; + } + + fd = mkstemp (tmpfilename); + if (fd == -1) { + perror ("mkstemp"); + return -1; + } + + snprintf (buf, sizeof buf, "/dev/fd/%d", fd); + + if (guestfs_download (g, filename, buf) == -1) { + close (fd); + return -1; + } + + if (close (fd) == -1) { + perror (tmpfilename); + return -1; + } + + if (asprintf (&outfile, "%s.out", tmpfilename) == -1) { + perror ("asprintf"); + return -1; + } + + /* Pass the expression to Perl via the environment. This sidesteps + * any quoting problems with the already complex Perl command line. + */ + setenv ("virt_edit_expr", perl_expr, 1); + + /* Call out to a canned Perl script. */ + if (asprintf (&cmd, + "perl -e '" + "$lineno = 0; " + "$expr = $ENV{virt_edit_expr}; " + "while (<STDIN>) { " + " $lineno++; " + " eval $expr; " + " die if $@; " + " print STDOUT $_ or die \"print: $!\"; " + "} " + "close STDOUT or die \"close: $!\"; " + "' < %s > %s", + tmpfilename, outfile) == -1) { + perror ("asprintf"); + return -1; + } + + r = system (cmd); + if (r == -1 || WEXITSTATUS (r) != 0) + return -1; + + if (rename (outfile, tmpfilename) == -1) { + perror ("rename"); + return -1; + } + + /* Upload to a new file in the same directory, so if it fails we + * don't end up with a partially written file. Give the new file + * a completely random name so we have only a tiny chance of + * overwriting some existing file. + */ + newname = generate_random_name (filename); + if (!newname) + return -1; + + /* Write new content. */ + if (guestfs_upload (g, tmpfilename, newname) == -1) + return -1; + + /* Set the permissions, UID, GID and SELinux context of the new + * file to match the old file (RHBZ#788641). + */ + if (guestfs_copy_attributes (g, filename, newname, + GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1) + return -1; + + /* Backup or overwrite the file. */ + if (backup_extension) { + CLEANUP_FREE char *backupname = NULL; + + backupname = generate_backup_name (filename, backup_extension); + if (backupname == NULL) + return -1; + + if (guestfs_mv (g, filename, backupname) == -1) + return -1; + } + if (guestfs_mv (g, newname, filename) == -1) + return -1; + + return 0; +} + static char random_char (void) { diff --git a/fish/file-edit.h b/fish/file-edit.h index 1d401c6..98fc453 100644 --- a/fish/file-edit.h +++ b/fish/file-edit.h @@ -32,4 +32,15 @@ extern int edit_file_editor (guestfs_h *g, const char *filename, const char *editor, const char *backup_extension); +/** + * Edit 'filename' running the specified 'perl_expr' using Perl. + * If 'backup_extension' is not null, then a copy of 'filename' is saved + * with 'backup_extension' appended to its file name. + * + * Returns -1 for failure, 0 otherwise (on success). + */ +extern int edit_file_perl (guestfs_h *g, const char *filename, + const char *perl_expr, + const char *backup_extension); + #endif -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 11/13] edit: switch to common editing functions
Switch virt-edit to the common edit_file_editor and edit_file_perl. --- edit/Makefile.am | 2 + edit/edit.c | 258 ++++--------------------------------------------------- 2 files changed, 19 insertions(+), 241 deletions(-) diff --git a/edit/Makefile.am b/edit/Makefile.am index cb19bec..9b1998b 100644 --- a/edit/Makefile.am +++ b/edit/Makefile.am @@ -28,6 +28,8 @@ bin_PROGRAMS = virt-edit SHARED_SOURCE_FILES = \ ../fish/config.c \ ../fish/domain.c \ + ../fish/file-edit.h \ + ../fish/file-edit.c \ ../fish/inspect.c \ ../fish/keys.c \ ../fish/options.h \ diff --git a/edit/edit.c b/edit/edit.c index 1f5ec7b..70ee129 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -38,6 +38,7 @@ #include "guestfs.h" #include "options.h" #include "windows.h" +#include "file-edit.h" /* Currently open libguestfs handle. */ guestfs_h *g; @@ -55,10 +56,6 @@ static const char *perl_expr = NULL; static void edit_files (int argc, char *argv[]); static void edit (const char *filename, const char *root); -static char *edit_interactively (const char *tmpfile); -static char *edit_non_interactively (const char *tmpfile); -static char *generate_random_name (const char *filename); -static char *generate_backup_name (const char *filename); static void __attribute__((noreturn)) usage (int status) @@ -324,15 +321,7 @@ static void edit (const char *filename, const char *root) { CLEANUP_FREE char *filename_to_free = NULL; - CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); - char tmpfile[strlen (tmpdir) + 32]; - sprintf (tmpfile, "%s/virteditXXXXXX", tmpdir); - - int fd; - char fdbuf[32]; - CLEANUP_FREE char *upload_from = NULL; - CLEANUP_FREE char *newname = NULL; - CLEANUP_FREE char *backupname = NULL; + int r; /* Windows? Special handling is required. */ if (root != NULL && is_windows (g, root)) { @@ -342,239 +331,26 @@ edit (const char *filename, const char *root) exit (EXIT_FAILURE); } - /* Download the file to a temporary. */ - fd = mkstemp (tmpfile); - if (fd == -1) { - perror ("mkstemp"); - exit (EXIT_FAILURE); - } - - snprintf (fdbuf, sizeof fdbuf, "/dev/fd/%d", fd); - - if (guestfs_download (g, filename, fdbuf) == -1) - goto error; - - if (close (fd) == -1) { - perror (tmpfile); - goto error; - } - - if (!perl_expr) - upload_from = edit_interactively (tmpfile); - else - upload_from = edit_non_interactively (tmpfile); - - /* We don't always need to upload: upload_from could be NULL because - * the user closed the editor without changing the file. - */ - if (upload_from) { - /* Upload to a new file in the same directory, so if it fails we - * don't end up with a partially written file. Give the new file - * a completely random name so we have only a tiny chance of - * overwriting some existing file. - */ - newname = generate_random_name (filename); - - if (guestfs_upload (g, upload_from, newname) == -1) - goto error; - - /* Set the permissions, UID, GID and SELinux context of the new - * file to match the old file (RHBZ#788641). - */ - if (guestfs_copy_attributes (g, filename, newname, - GUESTFS_COPY_ATTRIBUTES_ALL, 1, -1) == -1) - goto error; - - /* Backup or overwrite the file. */ - if (backup_extension) { - backupname = generate_backup_name (filename); - if (guestfs_mv (g, filename, backupname) == -1) - goto error; - } - if (guestfs_mv (g, newname, filename) == -1) - goto error; - } - - unlink (tmpfile); - return; - - error: - unlink (tmpfile); - exit (EXIT_FAILURE); -} - -static char * -edit_interactively (const char *tmpfile) -{ - struct utimbuf times; - struct stat oldstat, newstat; - const char *editor; - CLEANUP_FREE char *cmd = NULL; - int r; - char *ret; - - /* Set the time back a few seconds on the original file. This is so - * that if the user is very fast at editing, or if EDITOR is an - * automatic editor, then the edit might happen within the 1 second - * granularity of mtime, and we would think the file hasn't changed. - */ - if (stat (tmpfile, &oldstat) == -1) { - perror (tmpfile); - exit (EXIT_FAILURE); - } - - times.actime = oldstat.st_atime - 5; - times.modtime = oldstat.st_mtime - 5; - if (utime (tmpfile, ×) == -1) { - perror ("utimes"); - exit (EXIT_FAILURE); - } + if (perl_expr != NULL) { + r = edit_file_perl (g, filename, perl_expr, backup_extension); + } else { + const char *editor; - if (stat (tmpfile, &oldstat) == -1) { - perror (tmpfile); - exit (EXIT_FAILURE); - } - - editor = getenv ("EDITOR"); - if (editor == NULL) - editor = "vi"; + editor = getenv ("EDITOR"); + if (editor == NULL) + editor = "vi"; - if (asprintf (&cmd, "%s %s", editor, tmpfile) == -1) { - perror ("asprintf"); - exit (EXIT_FAILURE); + r = edit_file_editor (g, filename, editor, backup_extension); } - if (verbose) - fprintf (stderr, "%s\n", cmd); - - r = system (cmd); - if (r == -1 || WEXITSTATUS (r) != 0) + switch (r) { + case -1: exit (EXIT_FAILURE); - - if (stat (tmpfile, &newstat) == -1) { - perror (tmpfile); - exit (EXIT_FAILURE); - } - - if (oldstat.st_ctime == newstat.st_ctime && - oldstat.st_mtime == newstat.st_mtime) { + case 1: printf ("File not changed.\n"); - return NULL; - } - - ret = strdup (tmpfile); - if (!ret) { - perror ("strdup"); - exit (EXIT_FAILURE); + break; + default: + /* Success. */ + break; } - - return ret; -} - -/* Note that virt-builder uses exactly the same code .. in OCaml. */ -static char * -edit_non_interactively (const char *tmpfile) -{ - CLEANUP_FREE char *cmd = NULL, *outfile = NULL; - char *ret; - int r; - - assert (perl_expr != NULL); - - /* Pass the expression to Perl via the environment. This sidesteps - * any quoting problems with the already complex Perl command line. - */ - setenv ("virt_edit_expr", perl_expr, 1); - - /* Call out to a canned Perl script. */ - if (asprintf (&cmd, - "perl -e '" - "$lineno = 0; " - "$expr = $ENV{virt_edit_expr}; " - "while (<STDIN>) { " - " $lineno++; " - " eval $expr; " - " die if $@; " - " print STDOUT $_ or die \"print: $!\"; " - "} " - "close STDOUT or die \"close: $!\"; " - "' < %s > %s.out", - tmpfile, tmpfile) == -1) { - perror ("asprintf"); - exit (EXIT_FAILURE); - } - - if (verbose) - fprintf (stderr, "%s\n", cmd); - - r = system (cmd); - if (r == -1 || WEXITSTATUS (r) != 0) - exit (EXIT_FAILURE); - - if (asprintf (&outfile, "%s.out", tmpfile) == -1) { - perror ("asprintf"); - exit (EXIT_FAILURE); - } - - if (rename (outfile, tmpfile) == -1) { - perror ("rename"); - exit (EXIT_FAILURE); - } - - ret = strdup (tmpfile); - if (!ret) { - perror ("strdup"); - exit (EXIT_FAILURE); - } - - return ret; /* caller will free */ -} - -static char -random_char (void) -{ - char c[] = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; - return c[random () % (sizeof c - 1)]; -} - -static char * -generate_random_name (const char *filename) -{ - char *ret, *p; - size_t i; - - ret = malloc (strlen (filename) + 16); - if (!ret) { - perror ("malloc"); - exit (EXIT_FAILURE); - } - strcpy (ret, filename); - - p = strrchr (ret, '/'); - assert (p); - p++; - - /* Because of "+ 16" above, there should be enough space in the - * output buffer to write 8 random characters here. - */ - for (i = 0; i < 8; ++i) - *p++ = random_char (); - *p++ = '\0'; - - return ret; /* caller will free */ -} - -static char * -generate_backup_name (const char *filename) -{ - char *ret; - - assert (backup_extension != NULL); - - if (asprintf (&ret, "%s%s", filename, backup_extension) == -1) { - perror ("asprintf"); - exit (EXIT_FAILURE); - } - - return ret; /* caller will free */ } -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 12/13] fish: edit: add verbose parameter
--- edit/edit.c | 4 ++-- fish/edit.c | 2 +- fish/file-edit.c | 10 ++++++++-- fish/file-edit.h | 5 +++-- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/edit/edit.c b/edit/edit.c index 70ee129..f1c3b91 100644 --- a/edit/edit.c +++ b/edit/edit.c @@ -332,7 +332,7 @@ edit (const char *filename, const char *root) } if (perl_expr != NULL) { - r = edit_file_perl (g, filename, perl_expr, backup_extension); + r = edit_file_perl (g, filename, perl_expr, backup_extension, verbose); } else { const char *editor; @@ -340,7 +340,7 @@ edit (const char *filename, const char *root) if (editor == NULL) editor = "vi"; - r = edit_file_editor (g, filename, editor, backup_extension); + r = edit_file_editor (g, filename, editor, backup_extension, verbose); } switch (r) { diff --git a/fish/edit.c b/fish/edit.c index f43ef3b..0f41a88 100644 --- a/fish/edit.c +++ b/fish/edit.c @@ -58,7 +58,7 @@ run_edit (const char *cmd, size_t argc, char *argv[]) if (remotefilename == NULL) return -1; - r = edit_file_editor (g, remotefilename, editor, NULL); + r = edit_file_editor (g, remotefilename, editor, NULL, 0 /* not verbose */); return r == -1 ? -1 : 0; } diff --git a/fish/file-edit.c b/fish/file-edit.c index fa68626..ff36ac2 100644 --- a/fish/file-edit.c +++ b/fish/file-edit.c @@ -41,7 +41,7 @@ static char *generate_backup_name (const char *filename, int edit_file_editor (guestfs_h *g, const char *filename, const char *editor, - const char *backup_extension) + const char *backup_extension, int verbose) { CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); CLEANUP_UNLINK_FREE char *tmpfilename = NULL; @@ -105,6 +105,9 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, return -1; } + if (verbose) + fprintf (stderr, "%s\n", cmd); + r = system (cmd); if (r == -1 || WEXITSTATUS (r) != 0) { perror (cmd); @@ -161,7 +164,7 @@ edit_file_editor (guestfs_h *g, const char *filename, const char *editor, int edit_file_perl (guestfs_h *g, const char *filename, const char *perl_expr, - const char *backup_extension) + const char *backup_extension, int verbose) { CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g); CLEANUP_UNLINK_FREE char *tmpfilename = NULL; @@ -223,6 +226,9 @@ edit_file_perl (guestfs_h *g, const char *filename, const char *perl_expr, return -1; } + if (verbose) + fprintf (stderr, "%s\n", cmd); + r = system (cmd); if (r == -1 || WEXITSTATUS (r) != 0) return -1; diff --git a/fish/file-edit.h b/fish/file-edit.h index 98fc453..3f8b95e 100644 --- a/fish/file-edit.h +++ b/fish/file-edit.h @@ -30,7 +30,8 @@ * the file (e.g. the user closed the editor without saving). */ extern int edit_file_editor (guestfs_h *g, const char *filename, - const char *editor, const char *backup_extension); + const char *editor, const char *backup_extension, + int verbose); /** * Edit 'filename' running the specified 'perl_expr' using Perl. @@ -41,6 +42,6 @@ extern int edit_file_editor (guestfs_h *g, const char *filename, */ extern int edit_file_perl (guestfs_h *g, const char *filename, const char *perl_expr, - const char *backup_extension); + const char *backup_extension, int verbose); #endif -- 1.9.3
Pino Toscano
2014-Aug-28 13:21 UTC
[Libguestfs] [PATCH 13/13] customize: use the common perl file editing code
Wrap edit_file_perl to OCaml, and use it instead of the OCaml version of it. --- builder/Makefile.am | 2 ++ customize/Makefile.am | 5 +++- customize/perl_edit-c.c | 55 +++++++++++++++++++++++++++++++++++++++++++ customize/perl_edit.ml | 62 ++----------------------------------------------- po/POTFILES | 1 + sysprep/Makefile.am | 2 ++ v2v/Makefile.am | 2 ++ 7 files changed, 68 insertions(+), 61 deletions(-) create mode 100644 customize/perl_edit-c.c diff --git a/builder/Makefile.am b/builder/Makefile.am index 726ca60..0ca5db6 100644 --- a/builder/Makefile.am +++ b/builder/Makefile.am @@ -101,6 +101,7 @@ deps = \ $(top_builddir)/customize/hostname.cmx \ $(top_builddir)/customize/timezone.cmx \ $(top_builddir)/customize/firstboot.cmx \ + $(top_builddir)/customize/perl_edit-c.o \ $(top_builddir)/customize/perl_edit.cmx \ $(top_builddir)/customize/crypt-c.o \ $(top_builddir)/customize/crypt.cmx \ @@ -108,6 +109,7 @@ deps = \ $(top_builddir)/customize/customize_cmdline.cmx \ $(top_builddir)/customize/customize_run.cmx \ $(top_builddir)/fish/guestfish-uri.o \ + $(top_builddir)/fish/guestfish-file-edit.o \ index-scan.o \ index-struct.o \ index-parse.o \ diff --git a/customize/Makefile.am b/customize/Makefile.am index 3c81f34..90234a0 100644 --- a/customize/Makefile.am +++ b/customize/Makefile.am @@ -49,6 +49,7 @@ SOURCES = \ customize_main.ml \ password.ml \ password.mli \ + perl_edit-c.c \ perl_edit.ml \ perl_edit.mli \ random_seed.ml \ @@ -62,13 +63,15 @@ if HAVE_OCAML deps = \ $(top_builddir)/fish/guestfish-uri.o \ + $(top_builddir)/fish/guestfish-file-edit.o \ $(top_builddir)/mllib/common_gettext.cmx \ $(top_builddir)/mllib/common_utils.cmx \ $(top_builddir)/mllib/config.cmx \ $(top_builddir)/mllib/regedit.cmx \ $(top_builddir)/mllib/uri-c.o \ $(top_builddir)/mllib/uRI.cmx \ - crypt-c.o + crypt-c.o \ + perl_edit-c.o if HAVE_OCAMLOPT OBJECTS = $(deps) diff --git a/customize/perl_edit-c.c b/customize/perl_edit-c.c new file mode 100644 index 0000000..92126f9 --- /dev/null +++ b/customize/perl_edit-c.c @@ -0,0 +1,55 @@ +/* virt-customize - interface to edit_file_perl + * Copyright (C) 2014 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#include <caml/alloc.h> +#include <caml/memory.h> +#include <caml/mlvalues.h> + +#include "file-edit.h" + +/** + * We try to reuse the internals of the OCaml binding (for extracting + * the guestfs handle, and raising errors); hopefully this should be safe, + * as long as it's kept internal within the libguestfs sources. + */ +#include "../ocaml/guestfs-c.h" + +#pragma GCC diagnostic ignored "-Wmissing-prototypes" + +value +virt_customize_edit_file_perl (value verbosev, value guestfsv, value filev, + value exprv) +{ + CAMLparam4 (verbosev, guestfsv, filev, exprv); + int r; + guestfs_h *g; + + g = Guestfs_val (guestfsv); + r = edit_file_perl (g, String_val (filev), String_val (exprv), NULL, + Bool_val (verbosev)); + if (r == -1) + ocaml_guestfs_raise_error (g, "edit_file_perl"); + + CAMLreturn (Val_unit); +} diff --git a/customize/perl_edit.ml b/customize/perl_edit.ml index e44ff69..c734438 100644 --- a/customize/perl_edit.ml +++ b/customize/perl_edit.ml @@ -16,63 +16,5 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. *) -open Common_gettext.Gettext -open Common_utils - -open Printf - -(* Implement the --edit option. - * - * Code copied from virt-edit. - *) -let rec edit_file ~verbose (g : Guestfs.guestfs) file expr - let file_old = file ^ "~" in - g#rename file file_old; - - (* Download the file to a temporary. *) - let tmpfile = Filename.temp_file "vbedit" "" in - unlink_on_exit tmpfile; - g#download file_old tmpfile; - - do_perl_edit ~verbose g tmpfile expr; - - (* Upload the file. Unlike virt-edit we can afford to fail here - * so we don't need the temporary upload file. - *) - g#upload tmpfile file; - - (* However like virt-edit we do need to copy attributes. *) - g#copy_attributes ~all:true file_old file; - g#rm file_old - -and do_perl_edit ~verbose g file expr - (* Pass the expression to Perl via the environment. This sidesteps - * any quoting problems with the already complex Perl command line. - *) - Unix.putenv "virt_edit_expr" expr; - - (* Call out to a canned Perl script. *) - let cmd = sprintf "\ - perl -e ' - $lineno = 0; - $expr = $ENV{virt_edit_expr}; - while (<STDIN>) { - $lineno++; - eval $expr; - die if $@; - print STDOUT $_ or die \"print: $!\"; - } - close STDOUT or die \"close: $!\"; - ' < %s > %s.out" file file in - - if verbose then - eprintf "%s\n%!" cmd; - - let r = Sys.command cmd in - if r <> 0 then ( - eprintf (f_"virt-builder: error: could not evaluate Perl expression '%s'\n") - expr; - exit 1 - ); - - Unix.rename (file ^ ".out") file +external edit_file : verbose:bool -> Guestfs.guestfs -> string -> string -> unit + = "virt_customize_edit_file_perl" diff --git a/po/POTFILES b/po/POTFILES index a31cc88..59233c9 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -13,6 +13,7 @@ cat/log.c cat/ls.c cat/visit.c customize/crypt-c.c +customize/perl_edit-c.c daemon/9p.c daemon/acl.c daemon/augeas.c diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index 97166b5..7b88ef5 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -97,10 +97,12 @@ deps = \ $(top_builddir)/customize/hostname.cmx \ $(top_builddir)/customize/timezone.cmx \ $(top_builddir)/customize/firstboot.cmx \ + $(top_builddir)/customize/perl_edit-c.o \ $(top_builddir)/customize/perl_edit.cmx \ $(top_builddir)/customize/customize_cmdline.cmx \ $(top_builddir)/customize/customize_run.cmx \ $(top_builddir)/fish/guestfish-uri.o \ + $(top_builddir)/fish/guestfish-file-edit.o \ sysprep_operation.cmx \ $(patsubst %,sysprep_operation_%.cmx,$(operations)) \ main.cmx diff --git a/v2v/Makefile.am b/v2v/Makefile.am index e3a000a..f4baf65 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -64,10 +64,12 @@ SOURCES_ML = \ SOURCES_C = \ $(top_builddir)/fish/progress.c \ + $(top_builddir)/fish/file-edit.c \ $(top_builddir)/mllib/tty-c.c \ $(top_builddir)/mllib/progress-c.c \ $(top_builddir)/mllib/mkdtemp-c.c \ $(top_builddir)/customize/crypt-c.c \ + $(top_builddir)/customize/perl_edit-c.c \ utils-c.c \ xml-c.c \ domainxml-c.c -- 1.9.3
Richard W.M. Jones
2014-Aug-28 18:36 UTC
Re: [Libguestfs] [PATCH 06/13] fish: edit: improve the editor execution
On Thu, Aug 28, 2014 at 03:21:09PM +0200, Pino Toscano wrote:> /* Edit it. */ > - /* XXX Safe? */ > - snprintf (buf, sizeof buf, "%s %s", editor, filename); > + if (asprintf (&cmd, "%s %s", editor, tmpfilename) == -1) {filename -> tmpfilename. Is that right? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Possibly Parallel Threads
- [PATCH 00/13] code refactorings for tools
- Re: [PATCH 3/3] fish: fix btrfs subvolumes display in error case
- [PATCH] fish: improve formatting of help text of generated commands
- [PATCH 1/2] fish: small refactor of config reading code
- [PATCH v2 0/3] btrfs subvolumes display fix