[PATCH 00/13] code refactorings for tools
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
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
--- 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
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
--- 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
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
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
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
On Thursday 28 August 2014 19:36:20 Richard W.M. Jones wrote:
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?
This commit is "right" actually, what is wrong is the previous commit which apparently was an old version missing that rename (slipped through the refactoring of the refactoring...). Previous commit fixed, thanks. -- Pino Toscano
On Fri, Aug 29, 2014 at 10:45:58AM +0200, Pino Toscano wrote:
On Thursday 28 August 2014 19:36:20 Richard W.M. Jones wrote:
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?
This commit is "right" actually, what is wrong is the previous commit which apparently was an old version missing that rename (slipped through the refactoring of the refactoring...).
Previous commit fixed, thanks.
ACK series. 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
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
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
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
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
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
--- 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
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
participants (2)
-
Pino Toscano -
Richard W.M. Jones