From 9a20d1f99bd5dcd22e55300a5e29748486e585d7 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 13 Dec 2014 13:38:31 -0600 Subject: [PATCH 1/1] new internal image deletion helper Define a new helper function delete_wim_image() and use in wimlib_delete_image() and in the error path of wimlib_add_image_multisource(). This fixes a bug: current_image wasn't being reset to WIMLIB_NO_IMAGE in the error path of wimlib_add_image_multisource(). Bad things would happen if the library user then continued to use the same WIMStruct. --- include/wimlib/wim.h | 3 ++ src/add_image.c | 10 ++---- src/delete_image.c | 79 ++++++++++++++++++++++++++------------------ 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/include/wimlib/wim.h b/include/wimlib/wim.h index 412a9f7e..a6275fa0 100644 --- a/include/wimlib/wim.h +++ b/include/wimlib/wim.h @@ -136,6 +136,9 @@ for_image(WIMStruct *wim, int image, int (*visitor)(WIMStruct *)); extern int wim_checksum_unhashed_streams(WIMStruct *wim); +extern int +delete_wim_image(WIMStruct *wim, int image); + /* Internal open flags (pass to open_wim_as_WIMStruct(), not wimlib_open_wim()) */ #define WIMLIB_OPEN_FLAG_FROM_PIPE 0x80000000 diff --git a/src/add_image.c b/src/add_image.c index 0aed1deb..86ba9f0b 100644 --- a/src/add_image.c +++ b/src/add_image.c @@ -191,14 +191,8 @@ wimlib_add_image_multisource(WIMStruct *wim, return 0; out_delete_image: - /* Unsuccessful; rollback the WIM to its original state. */ - - /* wimlib_update_image() is now all-or-nothing, so no dentries remain - * and there's no need to pass the lookup table here. */ - put_image_metadata(wim->image_metadata[wim->hdr.image_count - 1], NULL); - - xml_delete_image(&wim->wim_info, wim->hdr.image_count); - wim->hdr.image_count--; + /* Unsuccessful; rollback by removing the new image. */ + delete_wim_image(wim, wim->hdr.image_count); return ret; } diff --git a/src/delete_image.c b/src/delete_image.c index 533d62cb..442f8b60 100644 --- a/src/delete_image.c +++ b/src/delete_image.c @@ -3,7 +3,7 @@ */ /* - * Copyright (C) 2012, 2013 Eric Biggers + * Copyright (C) 2012, 2013, 2014 Eric Biggers * * This file is free software; you can redistribute it and/or modify it under * the terms of the GNU Lesser General Public License as published by the Free @@ -23,13 +23,53 @@ # include "config.h" #endif +#include + #include "wimlib.h" -#include "wimlib/error.h" #include "wimlib/metadata.h" -#include "wimlib/util.h" #include "wimlib/wim.h" #include "wimlib/xml.h" +/* Internal method for single-image deletion. This doesn't set the + * image_deletion_occurred' flag on the WIMStruct. */ +int +delete_wim_image(WIMStruct *wim, int image) +{ + int ret; + + /* Load the metadata for the image to be deleted. This is necessary + * because streams referenced by files in the image need to have their + * reference counts decremented. */ + ret = select_wim_image(wim, image); + if (ret) + return ret; + + /* Release the reference to the image metadata and decrement reference + * counts on the streams referenced by files in the image. */ + put_image_metadata(wim->image_metadata[image - 1], wim->lookup_table); + + /* Remove the empty slot from the image metadata array. */ + memmove(&wim->image_metadata[image - 1], &wim->image_metadata[image], + (wim->hdr.image_count - image) * + sizeof(wim->image_metadata[0])); + + /* Decrement the image count. */ + --wim->hdr.image_count; + + /* Remove the image from the XML information. */ + xml_delete_image(&wim->wim_info, image); + + /* Fix the boot index. */ + if (wim->hdr.boot_idx == image) + wim->hdr.boot_idx = 0; + else if (wim->hdr.boot_idx > image) + wim->hdr.boot_idx--; + + /* The image is no longer valid. */ + wim->current_image = WIMLIB_NO_IMAGE; + return 0; +} + /* API function documented in wimlib.h */ WIMLIBAPI int wimlib_delete_image(WIMStruct *wim, int image) @@ -38,46 +78,19 @@ wimlib_delete_image(WIMStruct *wim, int image) int first, last; if (image == WIMLIB_ALL_IMAGES) { + /* Deleting all images */ last = wim->hdr.image_count; first = 1; } else { + /* Deleting one image */ last = image; first = image; } for (image = last; image >= first; image--) { - DEBUG("Deleting image %d", image); - - /* Even if the dentry tree is not allocated, we must select it - * (and therefore allocate it) so that we can decrement stream - * reference counts. */ - ret = select_wim_image(wim, image); + ret = delete_wim_image(wim, image); if (ret) return ret; - - /* Unless the image metadata is shared by another WIMStruct, - * free the dentry tree, free the security data, and decrement - * stream reference counts. */ - put_image_metadata(wim->image_metadata[image - 1], wim->lookup_table); - - /* Get rid of the empty slot in the image metadata array. */ - for (int i = image - 1; i < wim->hdr.image_count - 1; i++) - wim->image_metadata[i] = wim->image_metadata[i + 1]; - - /* Decrement the image count. */ - --wim->hdr.image_count; - - /* Fix the boot index. */ - if (wim->hdr.boot_idx == image) - wim->hdr.boot_idx = 0; - else if (wim->hdr.boot_idx > image) - wim->hdr.boot_idx--; - - wim->current_image = WIMLIB_NO_IMAGE; - - /* Remove the image from the XML information. */ - xml_delete_image(&wim->wim_info, image); - wim->image_deletion_occurred = 1; } return 0; -- 2.43.0