[libvirt] [PATCH 0/2] Remove and prohibit Windows special chars in filenames

After I fixed the first problem with a : in a filename a second instance occured shortly afterwards. The first patch fixes this problem and the second patch adds a syntax-check rule to prohibit such Windows special chars in filenames. Matthias

Windows doesn't allow : in filenames. Commit 6fdece9a332fc668a89bde96af94e7b7cbf6750d added files with a : in their names. This broke git operations on Windows as git is not able to create those files on clone or pull. Replace : with - in the offending filenames and adapt the test case. As the tested code expects the files to exist with : in the path use symlinks to provide the name that way. --- tests/virscsidata/0-0-0-0/block/sda/dev | 1 + tests/virscsidata/0-0-0-0/model | 1 + tests/virscsidata/0-0-0-0/scsi_generic/sg0/dev | 1 + tests/virscsidata/0-0-0-0/vendor | 1 + tests/virscsidata/0:0:0:0/block/sda/dev | 1 - tests/virscsidata/0:0:0:0/model | 1 - tests/virscsidata/0:0:0:0/scsi_generic/sg0/dev | 1 - tests/virscsidata/0:0:0:0/vendor | 1 - tests/virscsidata/1-0-0-0/block/sdh/dev | 1 + tests/virscsidata/1-0-0-0/model | 1 + tests/virscsidata/1-0-0-0/scsi_generic/sg8/dev | 1 + tests/virscsidata/1-0-0-0/vendor | 1 + tests/virscsidata/1:0:0:0/block/sdh/dev | 1 - tests/virscsidata/1:0:0:0/model | 1 - tests/virscsidata/1:0:0:0/scsi_generic/sg8/dev | 1 - tests/virscsidata/1:0:0:0/vendor | 1 - tests/virscsitest.c | 62 ++++++++++++++++++++++++++ 17 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 tests/virscsidata/0-0-0-0/block/sda/dev create mode 100644 tests/virscsidata/0-0-0-0/model create mode 100644 tests/virscsidata/0-0-0-0/scsi_generic/sg0/dev create mode 100644 tests/virscsidata/0-0-0-0/vendor delete mode 100644 tests/virscsidata/0:0:0:0/block/sda/dev delete mode 100644 tests/virscsidata/0:0:0:0/model delete mode 100644 tests/virscsidata/0:0:0:0/scsi_generic/sg0/dev delete mode 100644 tests/virscsidata/0:0:0:0/vendor create mode 100644 tests/virscsidata/1-0-0-0/block/sdh/dev create mode 100644 tests/virscsidata/1-0-0-0/model create mode 100644 tests/virscsidata/1-0-0-0/scsi_generic/sg8/dev create mode 100644 tests/virscsidata/1-0-0-0/vendor delete mode 100644 tests/virscsidata/1:0:0:0/block/sdh/dev delete mode 100644 tests/virscsidata/1:0:0:0/model delete mode 100644 tests/virscsidata/1:0:0:0/scsi_generic/sg8/dev delete mode 100644 tests/virscsidata/1:0:0:0/vendor diff --git a/tests/virscsidata/0-0-0-0/block/sda/dev b/tests/virscsidata/0-0-0-0/block/sda/dev new file mode 100644 index 0000000..fae0a50 --- /dev/null +++ b/tests/virscsidata/0-0-0-0/block/sda/dev @@ -0,0 +1 @@ +8:0 diff --git a/tests/virscsidata/0-0-0-0/model b/tests/virscsidata/0-0-0-0/model new file mode 100644 index 0000000..4d7e3ef --- /dev/null +++ b/tests/virscsidata/0-0-0-0/model @@ -0,0 +1 @@ +TOSHIBA MK5061GS diff --git a/tests/virscsidata/0-0-0-0/scsi_generic/sg0/dev b/tests/virscsidata/0-0-0-0/scsi_generic/sg0/dev new file mode 100644 index 0000000..992e920 --- /dev/null +++ b/tests/virscsidata/0-0-0-0/scsi_generic/sg0/dev @@ -0,0 +1 @@ +21:0 diff --git a/tests/virscsidata/0-0-0-0/vendor b/tests/virscsidata/0-0-0-0/vendor new file mode 100644 index 0000000..531030d --- /dev/null +++ b/tests/virscsidata/0-0-0-0/vendor @@ -0,0 +1 @@ +ATA diff --git a/tests/virscsidata/0:0:0:0/block/sda/dev b/tests/virscsidata/0:0:0:0/block/sda/dev deleted file mode 100644 index fae0a50..0000000 --- a/tests/virscsidata/0:0:0:0/block/sda/dev +++ /dev/null @@ -1 +0,0 @@ -8:0 diff --git a/tests/virscsidata/0:0:0:0/model b/tests/virscsidata/0:0:0:0/model deleted file mode 100644 index 4d7e3ef..0000000 --- a/tests/virscsidata/0:0:0:0/model +++ /dev/null @@ -1 +0,0 @@ -TOSHIBA MK5061GS diff --git a/tests/virscsidata/0:0:0:0/scsi_generic/sg0/dev b/tests/virscsidata/0:0:0:0/scsi_generic/sg0/dev deleted file mode 100644 index 992e920..0000000 --- a/tests/virscsidata/0:0:0:0/scsi_generic/sg0/dev +++ /dev/null @@ -1 +0,0 @@ -21:0 diff --git a/tests/virscsidata/0:0:0:0/vendor b/tests/virscsidata/0:0:0:0/vendor deleted file mode 100644 index 531030d..0000000 --- a/tests/virscsidata/0:0:0:0/vendor +++ /dev/null @@ -1 +0,0 @@ -ATA diff --git a/tests/virscsidata/1-0-0-0/block/sdh/dev b/tests/virscsidata/1-0-0-0/block/sdh/dev new file mode 100644 index 0000000..3d33f0f --- /dev/null +++ b/tests/virscsidata/1-0-0-0/block/sdh/dev @@ -0,0 +1 @@ +11:0 diff --git a/tests/virscsidata/1-0-0-0/model b/tests/virscsidata/1-0-0-0/model new file mode 100644 index 0000000..4d8a0e5 --- /dev/null +++ b/tests/virscsidata/1-0-0-0/model @@ -0,0 +1 @@ +DVDRAM GT50N diff --git a/tests/virscsidata/1-0-0-0/scsi_generic/sg8/dev b/tests/virscsidata/1-0-0-0/scsi_generic/sg8/dev new file mode 100644 index 0000000..bd84814 --- /dev/null +++ b/tests/virscsidata/1-0-0-0/scsi_generic/sg8/dev @@ -0,0 +1 @@ +21:1 diff --git a/tests/virscsidata/1-0-0-0/vendor b/tests/virscsidata/1-0-0-0/vendor new file mode 100644 index 0000000..d0383d4 --- /dev/null +++ b/tests/virscsidata/1-0-0-0/vendor @@ -0,0 +1 @@ +HL-DT-ST diff --git a/tests/virscsidata/1:0:0:0/block/sdh/dev b/tests/virscsidata/1:0:0:0/block/sdh/dev deleted file mode 100644 index 3d33f0f..0000000 --- a/tests/virscsidata/1:0:0:0/block/sdh/dev +++ /dev/null @@ -1 +0,0 @@ -11:0 diff --git a/tests/virscsidata/1:0:0:0/model b/tests/virscsidata/1:0:0:0/model deleted file mode 100644 index 4d8a0e5..0000000 --- a/tests/virscsidata/1:0:0:0/model +++ /dev/null @@ -1 +0,0 @@ -DVDRAM GT50N diff --git a/tests/virscsidata/1:0:0:0/scsi_generic/sg8/dev b/tests/virscsidata/1:0:0:0/scsi_generic/sg8/dev deleted file mode 100644 index bd84814..0000000 --- a/tests/virscsidata/1:0:0:0/scsi_generic/sg8/dev +++ /dev/null @@ -1 +0,0 @@ -21:1 diff --git a/tests/virscsidata/1:0:0:0/vendor b/tests/virscsidata/1:0:0:0/vendor deleted file mode 100644 index d0383d4..0000000 --- a/tests/virscsidata/1:0:0:0/vendor +++ /dev/null @@ -1 +0,0 @@ -HL-DT-ST diff --git a/tests/virscsitest.c b/tests/virscsitest.c index b917e47..b4ed5e9 100644 --- a/tests/virscsitest.c +++ b/tests/virscsitest.c @@ -25,11 +25,14 @@ #include "virscsi.h" #include "testutils.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_NONE #define VIR_SCSI_DATA "/virscsidata" +VIR_LOG_INIT("tests.scsitest"); + static const char *abs_top_srcdir; static char *virscsi_prefix = NULL; @@ -162,9 +165,38 @@ test2(const void *data ATTRIBUTE_UNUSED) } static int +create_symlink(const char *tmpdir, const char *src_name, const char *dst_name) +{ + int ret = -1; + char *src_path = NULL; + char *dst_path = NULL; + + if (virAsprintf(&src_path, "%s/%s", virscsi_prefix, src_name) < 0) + goto cleanup; + + if (virAsprintf(&dst_path, "%s/%s", tmpdir, dst_name) < 0) + goto cleanup; + + if (symlink(src_path, dst_path) < 0) { + VIR_WARN("Failed to create symlink '%s' to '%s'", src_path, dst_path); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(src_path); + VIR_FREE(dst_path); + + return ret; +} + +static int mymain(void) { int ret = 0; + char *tmpdir = NULL; + char template[] = "/tmp/libvirt_XXXXXX"; abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) @@ -175,12 +207,42 @@ mymain(void) goto cleanup; } + tmpdir = mkdtemp(template); + + if (tmpdir == NULL) { + VIR_WARN("Failed to create temporary directory"); + ret = -1; + goto cleanup; + } + +#define CREATE_SYMLINK(src_name, dst_name) \ + do { \ + if (create_symlink(tmpdir, src_name, dst_name) < 0) { \ + ret = -1; \ + goto cleanup; \ + } \ + } while (0) + + CREATE_SYMLINK("0-0-0-0", "0:0:0:0"); + CREATE_SYMLINK("1-0-0-0", "1:0:0:0"); + CREATE_SYMLINK("sg0", "sg0"); + CREATE_SYMLINK("sg8", "sg8"); + + VIR_FREE(virscsi_prefix); + + if (VIR_STRDUP(virscsi_prefix, tmpdir) < 0) { + ret = -1; + goto cleanup; + } + if (virtTestRun("test1", test1, NULL) < 0) ret = -1; if (virtTestRun("test2", test2, NULL) < 0) ret = -1; cleanup: + if (tmpdir && getenv("LIBVIRT_SKIP_CLEANUP") == NULL) + virFileDeleteTree(tmpdir); VIR_FREE(virscsi_prefix); return ret; } -- 1.8.1.2

Using any of these chars [:*?"<>|] in a filename is forbidden on Windows and breaks git operations on Windows as git is not able to create those files/directories on clone or pull. Because some of them can be used in UNIX filenames they tend to creep into filenames, especially : in PCI/SCSI device names that are used as filenames in test cases. --- cfg.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cfg.mk b/cfg.mk index a4ae978..117584c 100644 --- a/cfg.mk +++ b/cfg.mk @@ -912,6 +912,11 @@ sc_curly_braces_style: 'braces around function body, see' \ 'HACKING' 1>&2; exit 1; } || : +sc_prohibit_windows_special_chars_in_filename: + @files=$$($(VC_LIST_EXCEPT) | grep '[:*?"<>|]'); \ + test -n "$$files" && { echo '$(ME): Windows special chars' \ + 'in filename not allowed:' 1>&2; echo $$files 1>&2; exit 1; } || : + # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.8.1.2

On 03/29/2014 05:23 AM, Matthias Bolte wrote:
After I fixed the first problem with a : in a filename a second instance occured shortly afterwards. The first patch fixes this problem and the second patch adds a syntax-check rule to prohibit such Windows special chars in filenames.
ACK series; and worth having in 1.2.3. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

2014-03-29 15:09 GMT+01:00 Eric Blake <eblake@redhat.com>:
On 03/29/2014 05:23 AM, Matthias Bolte wrote:
After I fixed the first problem with a : in a filename a second instance occured shortly afterwards. The first patch fixes this problem and the second patch adds a syntax-check rule to prohibit such Windows special chars in filenames.
ACK series; and worth having in 1.2.3.
Thanks, pushed. -- Matthias Bolte http://photron.blogspot.com
participants (2)
-
Eric Blake
-
Matthias Bolte