From d8f4ab2d599956fa4511d4cb8a6b68e613cf239f Mon Sep 17 00:00:00 2001 From: Arystanbek Dyussenov Date: Thu, 23 Jul 2009 15:57:30 +0000 Subject: [PATCH] API: - freeing strings returned by RNA struct functions in RNA_parameter_list_free Unit tests: - check that BKE_export_image actually creates a file. This test is becoming dangerous: it creates and deletes files under /tmp. Having written this complicated test function I now realize it's much easier to write tests in a scripted language, which gives more freedom in expressions and need not be compiled. --- release/io/export_fbx.py | 56 ++++++++----- source/blender/blenkernel/intern/image.c | 23 +++-- source/blender/makesrna/intern/rna_access.c | 9 +- source/blender/python/intern/bpy_rna.c | 3 +- source/creator/tests/alltest.c | 93 +++++++++++++++++---- 5 files changed, 129 insertions(+), 55 deletions(-) diff --git a/release/io/export_fbx.py b/release/io/export_fbx.py index f98cefb076e..b7caf2b24dc 100644 --- a/release/io/export_fbx.py +++ b/release/io/export_fbx.py @@ -237,24 +237,24 @@ def sane_texname(data): return sane_name(data, sane_name_mapping_tex) def sane_takename(data): return sane_name(data, sane_name_mapping_take) def sane_groupname(data): return sane_name(data, sane_name_mapping_group) -def derived_paths(fname_orig, basepath, FORCE_CWD=False): - ''' - fname_orig - blender path, can be relative - basepath - fname_rel will be relative to this - FORCE_CWD - dont use the basepath, just add a ./ to the filename. - use when we know the file will be in the basepath. - ''' - fname = bpy.sys.expandpath(fname_orig) -# fname = Blender.sys.expandpath(fname_orig) - fname_strip = os.path.basename(fname) -# fname_strip = strip_path(fname) - if FORCE_CWD: - fname_rel = '.' + os.sep + fname_strip - else: - fname_rel = bpy.sys.relpath(fname, basepath) -# fname_rel = Blender.sys.relpath(fname, basepath) - if fname_rel.startswith('//'): fname_rel = '.' + os.sep + fname_rel[2:] - return fname, fname_strip, fname_rel +# def derived_paths(fname_orig, basepath, FORCE_CWD=False): +# ''' +# fname_orig - blender path, can be relative +# basepath - fname_rel will be relative to this +# FORCE_CWD - dont use the basepath, just add a ./ to the filename. +# use when we know the file will be in the basepath. +# ''' +# fname = bpy.sys.expandpath(fname_orig) +# # fname = Blender.sys.expandpath(fname_orig) +# fname_strip = os.path.basename(fname) +# # fname_strip = strip_path(fname) +# if FORCE_CWD: +# fname_rel = '.' + os.sep + fname_strip +# else: +# fname_rel = bpy.sys.relpath(fname, basepath) +# # fname_rel = Blender.sys.relpath(fname, basepath) +# if fname_rel.startswith('//'): fname_rel = '.' + os.sep + fname_rel[2:] +# return fname, fname_strip, fname_rel def mat4x4str(mat): @@ -1324,6 +1324,8 @@ def write(filename, batch_objects = None, \ # tex is an Image (Arystan) def write_video(texname, tex): + if not EXP_IMAGE_COPY: return + # Same as texture really! file.write('\n\tVideo: "Video::%s", "Clip" {' % texname) @@ -1335,7 +1337,10 @@ def write(filename, batch_objects = None, \ Property: "Width", "int", "",0 Property: "Height", "int", "",0''') if tex: - fname, fname_strip, fname_rel = derived_paths(tex.filename, basepath, EXP_IMAGE_COPY) + abspath = tex.export(basepath) + fname_rel = os.path.relpath(abspath, basepath) + fname_strip = os.path.basename(abspath) +# fname, fname_strip, fname_rel = derived_paths(tex.filename, basepath, EXP_IMAGE_COPY) else: fname = fname_strip = fname_rel = '' @@ -1361,6 +1366,8 @@ def write(filename, batch_objects = None, \ def write_texture(texname, tex, num): + if not EXP_IMAGE_COPY: return + # if tex == None then this is a dummy tex file.write('\n\tTexture: "Texture::%s", "TextureVideoClip" {' % texname) file.write('\n\t\tType: "TextureVideoClip"') @@ -1399,7 +1406,10 @@ def write(filename, batch_objects = None, \ file.write('\n\t\tMedia: "Video::%s"' % texname) if tex: - fname, fname_strip, fname_rel = derived_paths(tex.filename, basepath, EXP_IMAGE_COPY) + abspath = tex.export(basepath) + fname_rel = os.path.relpath(abspath, basepath) + fname_strip = os.path.basename(abspath) +# fname, fname_strip, fname_rel = derived_paths(tex.filename, basepath, EXP_IMAGE_COPY) else: fname = fname_strip = fname_rel = '' @@ -3074,9 +3084,9 @@ Takes: {''') # copy images if enabled - if EXP_IMAGE_COPY: -# copy_images( basepath, [ tex[1] for tex in textures if tex[1] != None ]) - bpy.util.copy_images( [ tex[1] for tex in textures if tex[1] != None ], basepath) +# if EXP_IMAGE_COPY: +# # copy_images( basepath, [ tex[1] for tex in textures if tex[1] != None ]) +# bpy.util.copy_images( [ tex[1] for tex in textures if tex[1] != None ], basepath) print('export finished in %.4f sec.' % (bpy.sys.time() - start_time)) # print 'export finished in %.4f sec.' % (Blender.sys.time() - start_time) diff --git a/source/blender/blenkernel/intern/image.c b/source/blender/blenkernel/intern/image.c index a469752602e..f0b29f766ec 100644 --- a/source/blender/blenkernel/intern/image.c +++ b/source/blender/blenkernel/intern/image.c @@ -2160,14 +2160,11 @@ int BKE_export_image(Image *im, const char *dest_dir, char *out_path, int out_pa /* expand "//" in filename and get absolute path */ BLI_convertstringcode(path, G.sce); - /* in unit tests, we don't want to modify the filesystem */ -#ifndef WITH_UNIT_TEST /* proceed only if image file exists */ if (!BLI_exists(path)) { if (G.f & G_DEBUG) printf("%s doesn't exist\n", path); - goto next; + return 0; } -#endif /* get the directory part */ BLI_split_dirfile_basic(path, dir, base); @@ -2191,10 +2188,8 @@ int BKE_export_image(Image *im, const char *dest_dir, char *out_path, int out_pa BLI_join_dirfile(dest_path, dest_dir, rel); -#ifndef WITH_UNIT_TEST /* build identical directory structure under dest_dir */ - BLI_make_existing_file(dest_path); -#endif + BLI_recurdir_fileops(dest_path); BLI_join_dirfile(dest_path, dest_path, base); } @@ -2206,14 +2201,18 @@ int BKE_export_image(Image *im, const char *dest_dir, char *out_path, int out_pa BLI_join_dirfile(dest_path, dest_dir, base); } -#ifndef WITH_UNIT_TEST if (G.f & G_DEBUG) printf("copying %s to %s\n", path, dest_path); - if (BLI_copy_fileops(path, dest_path) != 0) { - if (G.f & G_DEBUG) printf("couldn't copy %s to %s\n", path, dest_path); - return 0; + /* only copy if paths differ */ + if (strcmp(path, dest_path)) { + if (BLI_copy_fileops(path, dest_path) != 0) { + if (G.f & G_DEBUG) printf("couldn't copy %s to %s\n", path, dest_path); + return 0; + } + } + else if (G.f & G_DEBUG){ + printf("%s and %s are the same file\n", path, dest_path); } -#endif BLI_strncpy(out_path, dest_path, out_path_len); diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c index 9c133605f96..c9bf97fd274 100644 --- a/source/blender/makesrna/intern/rna_access.c +++ b/source/blender/makesrna/intern/rna_access.c @@ -2820,9 +2820,12 @@ void RNA_parameter_list_free(ParameterList *parms) if(parm->type == PROP_COLLECTION) { BLI_freelistN((ListBase*)((char*)parms->data+tot)); } - else if(parm->flag & PROP_DYNAMIC_ARRAY) { - /* for dynamic arrays, data is a pointer to an array */ - MEM_freeN(*(char**)parms->data+tot); + else if(parm == parms->func->ret) { + /* for dynamic arrays and strings, data is a pointer to an array */ + char *ptr= *(char**)((char*)parms->data+tot); + if((parm->flag & PROP_DYNAMIC_ARRAY || parm->type == PROP_STRING) && ptr) { + MEM_freeN(ptr); + } } tot+= rna_parameter_size(parm); diff --git a/source/blender/python/intern/bpy_rna.c b/source/blender/python/intern/bpy_rna.c index c7767c6c08e..780c58e2877 100644 --- a/source/blender/python/intern/bpy_rna.c +++ b/source/blender/python/intern/bpy_rna.c @@ -1844,7 +1844,8 @@ PyObject *pyrna_param_to_py(PointerRNA *ptr, PropertyRNA *prop, void *data) break; case PROP_STRING: { - ret = PyUnicode_FromString( *(char**)data ); + char *ptr = *(char**)data; + ret = ptr ? PyUnicode_FromString( ptr ) : Py_None; break; } case PROP_ENUM: diff --git a/source/creator/tests/alltest.c b/source/creator/tests/alltest.c index 2173ecd224f..8bb2b1a9bb0 100644 --- a/source/creator/tests/alltest.c +++ b/source/creator/tests/alltest.c @@ -10,6 +10,9 @@ #include "BKE_global.h" #include "BLI_listbase.h" +#include "BLI_util.h" +#include "BLI_fileops.h" +#include "BLI_string.h" #include "DNA_image_types.h" @@ -21,12 +24,31 @@ typedef struct ImageTestData { char *expect_path; /* file path that we expect */ int type; /* image type */ int ret; /* expected function return value */ + int create_file; /* whether the file should be created */ } ImageTestData; +/* recursively deletes a directory only if it is under /tmp */ +static void delete_only_tmp(char *path, int dir) { +#ifdef WIN32 +#else + if (!strncmp(path, "/tmp/", 5) && BLI_exists(path)) { + BLI_delete(path, dir, 1); + } +#endif +} + +static void touch_only_tmp(char *path) { +#ifdef WIN32 +#else + if (!strncmp(path, "/tmp/", 5)) { + BLI_touch(path); + } +#endif +} + /* check that BKE_copy_images manipulates paths correctly */ START_TEST(test_copy_images) { - char *dest_dir[] = {"/tmp/", "/tmp", NULL}; char **dir; ImageTestData *test; @@ -49,38 +71,77 @@ START_TEST(test_copy_images) if so, BKE_copy_images currently doesn't support them! */ static ImageTestData test_data[] = { - {"//bar/image.png", "/tmp/bar/image.png", IMA_TYPE_IMAGE, 1}, - {"/foo/bar/image.png", "/tmp/image.png", IMA_TYPE_IMAGE, 1}, - {"//image.png", "/tmp/image.png", IMA_TYPE_IMAGE, 1}, - {"//../../../foo/bar/image.png", "/tmp/image.png", IMA_TYPE_IMAGE, 1}, - {"//./foo/bar/image.png", "/tmp/foo/bar/image.png", IMA_TYPE_IMAGE, 1}, - {"/tmp/image.png", "/tmp/image.png", IMA_TYPE_IMAGE, 1}, - {"//textures/test/foo/bar/image.png", "/tmp/textures/test/foo/bar/image.png", IMA_TYPE_IMAGE, 1}, - {"//textures/test/foo/bar/image.png", "", IMA_TYPE_MULTILAYER, 0}, - {"", "", IMA_TYPE_IMAGE, 0}, + {"//bar/image.png", "/tmp/blender/dest/bar/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"//image.png", "/tmp/blender/dest/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"//textures/test/foo/bar/image.png", "/tmp/blender/dest/textures/test/foo/bar/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"//textures/test/foo/bar/image.png", "", IMA_TYPE_MULTILAYER, 0, 1}, + {"//./foo/bar/image.png", "/tmp/blender/dest/foo/bar/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"//../foo/bar/image.png", "/tmp/blender/dest/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"/tmp/blender/image.png", "/tmp/blender/dest/image.png", IMA_TYPE_IMAGE, 1, 1}, + /* expecting it to return 1 when src and dest are the same file */ + {"/tmp/blender/foo/bar/image.png", "/tmp/blender/dest/image.png", IMA_TYPE_IMAGE, 1, 1}, + {"/tmp/blender/dest/image.png", "/tmp/blender/dest/image.png", IMA_TYPE_IMAGE, 1, 1}, + /* expecting empty path and 0 return value for non-existing files */ + {"/tmp/blender/src/file-not-created", "", IMA_TYPE_IMAGE, 0, 0}, + {"", "", IMA_TYPE_IMAGE, 0, 0}, {NULL, NULL}, }; + char *dest_dir[] = {"/tmp/blender/dest/", "/tmp/blender/dest", NULL}; + const char *blend_dir = "/tmp/blender/src"; + /* substitute G.sce */ - BLI_strncpy(G.sce, "/tmp/foo/bar/untitled.blend", sizeof(G.sce)); + BLI_snprintf(G.sce, sizeof(G.sce), "%s/untitled.blend", blend_dir); #endif + /* only delete files/directories under /tmp/ ! */ + delete_only_tmp(blend_dir, 1); + + for (dir = dest_dir; *dir; dir++) { + delete_only_tmp(*dir, 1); + } + + /* create files */ + BLI_recurdir_fileops(blend_dir); + + /* create fake empty source files */ + for (test= &test_data[0]; test->path; test++) { + char dir[FILE_MAX]; + char path[FILE_MAX]; + + if (!test->create_file) continue; + + /* expand "//" */ + BLI_strncpy(path, test->path, sizeof(path)); + BLI_convertstringcode(path, G.sce); + + /* create a directory */ + BLI_split_dirfile_basic(path, dir, NULL); + BLI_recurdir_fileops(dir); + + /* create a file */ + touch_only_tmp(path); + } + for (dir = dest_dir; *dir; dir++) { for (test= &test_data[0]; test->path; test++) { Image image; char path[FILE_MAX]; + char part[200]; int ret; BLI_strncpy(image.name, test->path, sizeof(image.name)); image.type= test->type; - ret= BKE_export_image(&image, *dest_dir, path, sizeof(path)); + ret= BKE_export_image(&image, *dir, path, sizeof(path)); /* check if we got correct output */ - fail_if(ret != test->ret, "For image with filename %s and type %d, expected %d as return value got %d.", - test->path, test->type, test->ret, ret); - fail_if(strcmp(path, test->expect_path), "For image with filename %s and type %d, expected path '%s' got '%s'.", - test->path, test->type, test->expect_path, path); + BLI_snprintf(part, sizeof(part), "For image with filename %s and type %d", test->path, test->type); + fail_if(ret != test->ret, "%s, expected %d as return value got %d.", part, test->ret, ret); + fail_if(strcmp(path, test->expect_path), "%s, expected path %s got \"%s\".", part, test->expect_path, path); + if (test->ret == ret && ret == 1) { + fail_if(!BLI_exists(test->expect_path), "%s, expected %s to be created.", part, test->expect_path); + } } } }