[Bf-blender-cvs] [624b231ec49] master: Cleanup: early out on invalid ID data to simplify control flow
Brecht Van Lommel
noreply at git.blender.org
Tue Apr 7 13:23:36 CEST 2020
Commit: 624b231ec49997120cff17ae68a39cb13e267ee7
Author: Brecht Van Lommel
Date: Fri Apr 3 14:12:57 2020 +0200
Branches: master
https://developer.blender.org/rB624b231ec49997120cff17ae68a39cb13e267ee7
Cleanup: early out on invalid ID data to simplify control flow
Differential Revision: https://developer.blender.org/D7325
===================================================================
M source/blender/blenloader/intern/readfile.c
===================================================================
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 3c5908efa3f..8f77c80d64b 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -9413,6 +9413,26 @@ static BHead *read_libblock(FileData *fd,
{
/* This routine reads a libblock and its direct data. Lib link functions will
* set points between datablocks. */
+ if (r_id) {
+ *r_id = NULL; /* In case of early return. */
+ }
+
+ /* Read libblock struct. */
+ fd->are_memchunks_identical = true;
+ ID *id = read_struct(fd, bhead, "lib block");
+ if (id == NULL) {
+ return blo_bhead_next(fd, bhead);
+ }
+
+ /* Determine ID type. */
+ const short idcode = GS(id->name);
+ ListBase *lb = which_libbase(main, idcode);
+ if (lb == NULL) {
+ /* Unknown ID type. */
+ printf("%s: unknown id code '%c%c'\n", __func__, (idcode & 0xff), (idcode >> 8));
+ MEM_freeN(id);
+ return blo_bhead_next(fd, bhead);
+ }
/* In undo case, most libs and linked data should be kept as is from previous state
* (see BLO_read_from_memfile).
@@ -9422,17 +9442,15 @@ static BHead *read_libblock(FileData *fd,
* That means we have to carefully check whether current lib or
* libdata already exits in old main, if it does we merely copy it over into new main area,
* otherwise we have to do a full read of that bhead... */
- if (fd->memfile && ELEM(bhead->code, ID_LI, ID_LINK_PLACEHOLDER)) {
- const char *idname = blo_bhead_id_name(fd, bhead);
-
- DEBUG_PRINTF("Checking %s...\n", idname);
+ if (fd->memfile != NULL && ELEM(bhead->code, ID_LI, ID_LINK_PLACEHOLDER)) {
+ DEBUG_PRINTF("Checking %s...\n", id->name);
if (bhead->code == ID_LI) {
Main *libmain = fd->old_mainlist->first;
/* Skip oldmain itself... */
for (libmain = libmain->next; libmain; libmain = libmain->next) {
DEBUG_PRINTF("... against %s: ", libmain->curlib ? libmain->curlib->id.name : "<NULL>");
- if (libmain->curlib && STREQ(idname, libmain->curlib->id.name)) {
+ if (libmain->curlib && STREQ(id->name, libmain->curlib->id.name)) {
Main *oldmain = fd->old_mainlist->first;
DEBUG_PRINTF("FOUND!\n");
/* In case of a library, we need to re-add its main to fd->mainlist,
@@ -9445,9 +9463,7 @@ static BHead *read_libblock(FileData *fd,
BLI_addtail(fd->mainlist, libmain);
BLI_addtail(&main->libraries, libmain->curlib);
- if (r_id) {
- *r_id = NULL; /* Just in case... */
- }
+ MEM_freeN(id);
return blo_bhead_next(fd, bhead);
}
DEBUG_PRINTF("nothing...\n");
@@ -9457,20 +9473,18 @@ static BHead *read_libblock(FileData *fd,
DEBUG_PRINTF("... in %s (%s): ",
main->curlib ? main->curlib->id.name : "<NULL>",
main->curlib ? main->curlib->name : "<NULL>");
- ID *id = BKE_libblock_find_name(main, GS(idname), idname + 2);
- if (id != NULL) {
+ ID *existing_id = BKE_libblock_find_name(main, GS(id->name), id->name + 2);
+ if (existing_id != NULL) {
DEBUG_PRINTF("FOUND!\n");
/* Even though we found our linked ID,
* there is no guarantee its address is still the same. */
- if (id != bhead->old) {
- oldnewmap_insert(fd->libmap, bhead->old, id, GS(id->name));
+ if (existing_id != bhead->old) {
+ oldnewmap_insert(fd->libmap, bhead->old, existing_id, GS(existing_id->name));
}
/* No need to do anything else for ID_LINK_PLACEHOLDER,
* it's assumed already present in its lib's main. */
- if (r_id) {
- *r_id = NULL; /* Just in case... */
- }
+ MEM_freeN(id);
return blo_bhead_next(fd, bhead);
}
DEBUG_PRINTF("nothing...\n");
@@ -9478,173 +9492,158 @@ static BHead *read_libblock(FileData *fd,
}
/* read libblock */
- fd->are_memchunks_identical = true;
- ID *id = read_struct(fd, bhead, "lib block");
- const short idcode = id != NULL ? GS(id->name) : 0;
-
BHead *id_bhead = bhead;
+
+ if (id_bhead->code != ID_LINK_PLACEHOLDER) {
+ /* need a name for the mallocN, just for debugging and sane prints on leaks */
+ const char *allocname = dataname(idcode);
+
+ /* read all data into fd->datamap */
+ /* TODO: for the undo case instead of building oldnewmap here we could just quickly check the
+ * bheads... could save some more ticks. Probably not worth it though, bottleneck is full
+ * depsgraph rebuild and evaluate, not actual file reading. */
+ bhead = read_data_into_oldnewmap(fd, id_bhead, allocname);
+ }
+
+ /* Restore existing datablocks for undo. */
+ const bool do_partial_undo = (fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0;
+
/* Used when undoing from memfile, we swap changed IDs into their old addresses when found. */
ID *id_old = NULL;
bool do_id_swap = false;
- if (id != NULL) {
- const bool do_partial_undo = (fd->skip_flags & BLO_READ_SKIP_UNDO_OLD_MAIN) == 0;
-
+ if (fd->memfile != NULL) {
if (id_bhead->code != ID_LINK_PLACEHOLDER) {
- /* need a name for the mallocN, just for debugging and sane prints on leaks */
- const char *allocname = dataname(idcode);
-
- /* read all data into fd->datamap */
- /* TODO: instead of building oldnewmap here we could just quickly check the bheads... could
- * save some more ticks. Probably not worth it though, bottleneck is full depsgraph rebuild
- * and evaluate, not actual file reading. */
- bhead = read_data_into_oldnewmap(fd, id_bhead, allocname);
-
DEBUG_PRINTF(
"%s: ID %s is unchanged: %d\n", __func__, id->name, fd->are_memchunks_identical);
- if (fd->memfile != NULL) {
- BLI_assert(fd->old_idmap != NULL || !do_partial_undo);
- /* This code should only ever be reached for local data-blocks. */
- BLI_assert(main->curlib == NULL);
-
- /* Find the 'current' existing ID we want to reuse instead of the one we would read from
- * the undo memfile. */
- DEBUG_PRINTF("\t Looking for ID %s with uuid %u instead of newly read one\n",
- id->name,
- id->session_uuid);
- id_old = do_partial_undo ? BKE_main_idmap_lookup_uuid(fd->old_idmap, id->session_uuid) :
- NULL;
- bool can_finalize_and_return = false;
-
- if (ELEM(idcode, ID_WM, ID_SCR, ID_WS)) {
- /* Read WindowManager, Screen and WorkSpace IDs are never actually used during undo (see
- * `setup_app_data()` in `blendfile.c`).
- * So we can just abort here, just ensuring libmapping is set accordingly. */
- can_finalize_and_return = true;
- }
- else if (id_old != NULL && fd->are_memchunks_identical) {
- /* Do not add LIB_TAG_NEW here, this should not be needed/used in undo case anyway (as
- * this is only for do_version-like code), but for sake of consistency, and also because
- * it will tell us which ID is re-used from old Main, and which one is actually new. */
- id_old->tag = tag | LIB_TAG_NEED_LINK | LIB_TAG_UNDO_OLD_ID_REUSED;
- id_old->lib = main->curlib;
- id_old->us = ID_FAKE_USERS(id_old);
- /* Do not reset id->icon_id here, memory allocated for it remains valid. */
- /* Needed because .blend may have been saved with crap value here... */
- id_old->newid = NULL;
- id_old->orig_id = NULL;
-
- /* About recalc: since that ID did not change at all, we know that its recalc fields also
- * remained unchanged, so no need to handle neither recalc nor recalc_undo_future here.
- */
+ BLI_assert(fd->old_idmap != NULL || !do_partial_undo);
+ /* This code should only ever be reached for local data-blocks. */
+ BLI_assert(main->curlib == NULL);
+
+ /* Find the 'current' existing ID we want to reuse instead of the one we would read from
+ * the undo memfile. */
+ DEBUG_PRINTF("\t Looking for ID %s with uuid %u instead of newly read one\n",
+ id->name,
+ id->session_uuid);
+ id_old = do_partial_undo ? BKE_main_idmap_lookup_uuid(fd->old_idmap, id->session_uuid) :
+ NULL;
+ bool can_finalize_and_return = false;
+
+ if (ELEM(idcode, ID_WM, ID_SCR, ID_WS)) {
+ /* Read WindowManager, Screen and WorkSpace IDs are never actually used during undo (see
+ * `setup_app_data()` in `blendfile.c`).
+ * So we can just abort here, just ensuring libmapping is set accordingly. */
+ can_finalize_and_return = true;
+ }
+ else if (id_old != NULL && fd->are_memchunks_identical) {
+ /* Do not add LIB_TAG_NEW here, this should not be needed/used in undo case anyway (as
+ * this is only for do_version-like code), but for sake of consistency, and also because
+ * it will tell us which ID is re-used from old Main, and which one is actually new. */
+ id_old->tag = tag | LIB_TAG_NEED_LINK | LIB_TAG_UNDO_OLD_ID_REUSED;
+ id_old->lib = main->curlib;
+ id_old->us = ID_FAKE_USERS(id_old);
+ /* Do not reset id->icon_id here, memory allocated for it remains valid. */
+ /* Needed because .blend may have been saved with crap value here... */
+ id_old->newid = NULL;
+ id_old->orig_id = NULL;
+
+ /* About recalc: since that ID did not change at all, we know that its recalc fields also
+ * remained unchanged, so no need to handle neither recalc nor recalc_undo_future here.
+ */
- Main *old_bmain = fd->old_mainlist->first;
- ListBase *old_lb = which_libbase(old_bmain, idcode);
- ListBase *new_lb = which_libbase(main, idcode);
- BLI_remlink(old_lb, id_old);
- BLI_addtail(new_lb, id_old);
+ Main *old_bmain = fd->old_mainlist->first;
+ ListBase *old_lb = which_libbase(old_bmain, idcode);
+ ListBase *new_lb = which_libbase(main, idcode);
+ BLI_remlink(old_lb, id_old);
+ BL
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list