new internal image deletion helper
authorEric Biggers <ebiggers3@gmail.com>
Sat, 13 Dec 2014 19:38:31 +0000 (13:38 -0600)
committerEric Biggers <ebiggers3@gmail.com>
Sun, 14 Dec 2014 04:43:22 +0000 (22:43 -0600)
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
src/add_image.c
src/delete_image.c

index 412a9f7e5aef988e16f10b958418c7d28d785b9b..a6275fa0669764d71dbc2a3b13939b7af571eb73 100644 (file)
@@ -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
index 0aed1deb3a11d5ce2bd868783b641f32f8672742..86ba9f0b3d277ede7bb588ba2baf57e754783907 100644 (file)
@@ -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;
 }
 
index 533d62cb502675dfaf0a94ec8c8d3b3d6456ff1d..442f8b609436f1c1c9c3e50eaa08c9eebaf2afc9 100644 (file)
@@ -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
 #  include "config.h"
 #endif
 
+#include <string.h>
+
 #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;