Hi Christian, I did not know this was for the 5 branch. Where was this discussed?
Functional changes to a release branch need to be treated carefully and in this case I would not approve it. I am sorry if I did not pick up it was for 5 as well. Thanks Chris On 9/12/21 6:21 pm, Christian Mauderer wrote: > Module: rtems > Branch: 5 > Commit: ff3f3490df7120c9ec039b5acd1935265c3f9262 > Changeset: > http://git.rtems.org/rtems/commit/?id=ff3f3490df7120c9ec039b5acd1935265c3f9262 > > Author: Christian Mauderer <christian.maude...@embedded-brains.de> > Date: Wed Dec 1 16:39:46 2021 +0100 > > untar: Make behavior similar to GNU or BSD tar > > RTEMS untar implementation had problems with overwriting or integrating > archives into existing directory structures. This patch adapts the > behavior to mimic that of a GNU tar or BSD tar and extends the tar01 > test to check for the behavior. That is: > > * If a directory structure exists, the files from the archive will be > integrated. Existing files are overwritten. > > * If a file exists and the archive contains a directory with the same > name, the file is removed and a directory is created. In the above > example: if l1/l2 is a file it will be overwritten with a new > directory. > > * If a directory exists and the archive contains a file with the same > name, the directory will be replaced if it is empty. If it contains > files, the result is an error. > > * An archive also can contain only a file without the parent > directories. If in that case one of the parent directories exists as a > file extracting the archive results in an error. In the example: if > l1/l2 is a file and the archive doesn't contain the directories but > only the file l1/l2/x.txt that would be an error. > > * In case of an error, it is possible that the archive has been > partially extracted. > > Closes #4552 > > --- > > cpukit/libmisc/untar/untar.c | 57 +++++++---- > testsuites/libtests/tar01/init.c | 199 > +++++++++++++++++++++++++++++++++++- > testsuites/libtests/tar01/tar01.doc | 1 + > testsuites/libtests/tar01/tar01.scn | 54 ++++++++-- > testsuites/libtests/tar01/tar01.tar | Bin 10240 -> 10240 bytes > 5 files changed, 282 insertions(+), 29 deletions(-) > > diff --git a/cpukit/libmisc/untar/untar.c b/cpukit/libmisc/untar/untar.c > index a2f09fb..8888ab2 100644 > --- a/cpukit/libmisc/untar/untar.c > +++ b/cpukit/libmisc/untar/untar.c > @@ -126,30 +126,25 @@ Make_Path(const rtems_printer *printer, char *path) > > *p = '\0'; > if (p[1] == '\0') { > - /* Speculatively unlink the last component so that it can be > re-created */ > - unlink(path); > return 0; > } > > if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO) != 0) { > - if (errno == EEXIST || errno == EISDIR) { > + if (errno == EEXIST) { > + /* If it exists already: Check whether it is a directory */ > struct stat sb; > - > - if (stat(path, &sb) != 0) { > + if (lstat(path, &sb) != 0) { > + Print_Error(printer, "lstat", path); > + return -1; > + } else if (!S_ISDIR(sb.st_mode)) { > + rtems_printf(printer, > + "untar: mkdir: %s: exists but is not a directory\n", > + path); > return -1; > } > - > - if (!S_ISDIR(sb.st_mode)) { > - if (unlink(path) != 0) { > - Print_Error(printer, "unlink", path); > - return -1; > - } > - > - if (mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO) != 0) { > - Print_Error(printer, "mkdir (unlink)", path); > - return -1; > - } > - } > + } else { > + Print_Error(printer, "mkdir", path); > + return -1; > } > } > > @@ -206,6 +201,12 @@ Untar_ProcessHeader( > > if (Make_Path(ctx->printer, ctx->file_path) != 0) { > retval = UNTAR_FAIL; > + } else { > + /* > + * Speculatively unlink. This should unlink everything but non-empty > + * directories or write protected stuff. > + */ > + unlink(ctx->file_path); > } > > if (ctx->linkflag == SYMTYPE) { > @@ -225,8 +226,22 @@ Untar_ProcessHeader( > rtems_printf(ctx->printer, "untar: dir: %s\n", ctx->file_path); > r = mkdir(ctx->file_path, ctx->mode); > if (r != 0) { > - Print_Error(ctx->printer, "mkdir", ctx->file_path); > - retval = UNTAR_FAIL; > + if (errno == EEXIST) { > + /* If it exists already: Check whether it is a directory */ > + struct stat sb; > + if (lstat(ctx->file_path, &sb) != 0) { > + Print_Error(ctx->printer, "lstat", ctx->file_path); > + retval = UNTAR_FAIL; > + } else if (!S_ISDIR(sb.st_mode)) { > + rtems_printf(ctx->printer, > + "untar: mkdir: %s: exists but is not a directory\n", > + ctx->file_path); > + retval = UNTAR_FAIL; > + } > + } else { > + Print_Error(ctx->printer, "mkdir", ctx->file_path); > + retval = UNTAR_FAIL; > + } > } > } > > @@ -426,7 +441,9 @@ Untar_FromFile_Print( > */ > > if ((out_fd = creat(ctx.file_path, ctx.mode)) == -1) { > - (void) lseek(fd, SEEK_CUR, 512UL * ctx.nblocks); > + /* Couldn't create that file. Abort. */ > + retval = UNTAR_FAIL; > + break; > } else { > for (i = 0; i < ctx.nblocks; i++) { > n = read(fd, bufr, 512); > diff --git a/testsuites/libtests/tar01/init.c > b/testsuites/libtests/tar01/init.c > index 4cad67a..2deff3a 100644 > --- a/testsuites/libtests/tar01/init.c > +++ b/testsuites/libtests/tar01/init.c > @@ -7,6 +7,33 @@ > * http://www.rtems.org/license/LICENSE. > */ > > +/* > + * Note on the used tar file: Generate the file on a system that supports > + * symlinks with the following commands (tested on Linux - you might have to > + * adapt on other systems): > + * > + * export WORK=some_work_directory > + * rm -r ${WORK} > + * mkdir -p ${WORK}/home/abc/def > + * mkdir -p ${WORK}/home/dir > + * cd ${WORK} > + * echo "#! joel" > home/abc/def/test_script > + * echo "ls -las /dev" >> home/abc/def/test_script > + * chmod 755 home/abc/def/test_script > + * echo "This is a test of loading an RTEMS filesystem from an" > > home/test_file > + * echo "initial tar image." >> home/test_file > + * echo "Hello world" >> home/dir/file > + * ln -s home/test_file symlink > + * tar cf tar01.tar --format=ustar \ > + * symlink \ > + * home/test_file \ > + * home/abc/def/test_script \ > + * home/dir > + * > + * Note that "home/dir" is in the archive as separate directory. "home/abc" > is > + * only in the archive as a parent of the file "test_script". > + */ > + > #ifdef HAVE_CONFIG_H > #include "config.h" > #endif > @@ -95,6 +122,84 @@ void test_untar_from_memory(void) > > } > > +static void assert_file_content( > + const char *name, > + const char *expected_content, > + ssize_t expected_size > +) > +{ > + char buf[16]; > + int fd; > + int rd; > + > + fd = open(name, O_RDONLY); > + rtems_test_assert( fd >= 0 ); > + do { > + rd = read(fd, buf, sizeof(buf)); > + rtems_test_assert( rd >= 0 ); > + if (rd > 0) { > + rtems_test_assert( expected_size - rd >= 0 ); > + rtems_test_assert( memcmp(buf, expected_content, rd) == 0 ); > + expected_content += rd; > + expected_size -= rd; > + } > + } while(rd > 0); > + rtems_test_assert( expected_size == 0 ); > + close(fd); > +} > + > +static void assert_content_like_expected(void) > +{ > + const char *directories[] = { > + "home", > + "home/abc", > + "home/abc/def", > + "home/dir", > + }; > + const char *symlinks[] = { > + "symlink", > + }; > + const struct { > + const char *name; > + const char *content; > + } files[] = { > + { > + .name = "home/abc/def/test_script", > + .content = "#! joel\nls -las /dev\n", > + }, { > + .name = "home/test_file", > + .content = "This is a test of loading an RTEMS filesystem from an\n" > + "initial tar image.\n", > + }, { > + .name = "home/dir/file", > + .content = "Hello world\n", > + } > + }; > + size_t i; > + struct stat st; > + > + for(i = 0; i < RTEMS_ARRAY_SIZE(directories); ++i) { > + lstat(directories[i], &st); > + rtems_test_assert( S_ISDIR(st.st_mode) ); > + } > + > + for(i = 0; i < RTEMS_ARRAY_SIZE(symlinks); ++i) { > + lstat(symlinks[i], &st); > + rtems_test_assert( S_ISLNK(st.st_mode) ); > + } > + > + for(i = 0; i < RTEMS_ARRAY_SIZE(files); ++i) { > + lstat(files[i].name, &st); > + rtems_test_assert( S_ISREG(st.st_mode) ); > + > + assert_file_content( > + files[i].name, > + files[i].content, > + strlen(files[i].content) > + ); > + } > +} > + > void test_untar_from_file(void) > { > int fd; > @@ -119,13 +224,105 @@ void test_untar_from_file(void) > rv = chdir( "/dest" ); > rtems_test_assert( rv == 0 ); > > - /* Untar it */ > + /* Case 1: Untar it into empty directory */ > rv = Untar_FromFile( "/test.tar" ); > printf("Untaring from file - "); > if (rv != UNTAR_SUCCESSFUL) { > printf ("error: untar failed: %i\n", rv); > exit(1); > } > + assert_content_like_expected(); > + printf ("successful\n"); > + > + /* Case 2: Most files exist */ > + rv = unlink("/dest/home/test_file"); > + rtems_test_assert( rv == 0 ); > + > + rv = Untar_FromFile( "/test.tar" ); > + printf("Untar from file into existing structure with one missing file - "); > + if (rv != UNTAR_SUCCESSFUL) { > + printf ("error: untar failed: %i\n", rv); > + exit(1); > + } > + assert_content_like_expected(); > + printf ("successful\n"); > + > + /* Case 3: An empty directory exists where a file should be */ > + rv = unlink("/dest/home/test_file"); > + rtems_test_assert( rv == 0 ); > + rv = mkdir("/dest/home/test_file", S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); > + rtems_test_assert( rv == 0 ); > + > + rv = Untar_FromFile( "/test.tar" ); > + printf("Untar from file; overwrite empty directory with file - "); > + if (rv != UNTAR_SUCCESSFUL) { > + printf ("error: untar failed: %i\n", rv); > + exit(1); > + } > + assert_content_like_expected(); > + printf ("successful\n"); > + > + /* Case 4: A file exists where a parent directory should be created */ > + rv = unlink("/dest/home/abc/def/test_script"); > + rtems_test_assert( rv == 0 ); > + rv = unlink("/dest/home/abc/def"); > + rtems_test_assert( rv == 0 ); > + rv = unlink("/dest/home/abc"); > + rtems_test_assert( rv == 0 ); > + fd = creat("/dest/home/abc", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > + rtems_test_assert( fd >= 0 ); > + close(fd); > + > + rv = Untar_FromFile( "/test.tar" ); > + printf("Untar from file; file exists where parent dir should be created - > "); > + if (rv != UNTAR_FAIL) { > + printf ("error: untar didn't fail like expected: %i\n", rv); > + exit(1); > + } > + printf ("expected fail\n"); > + /* cleanup so that the next one works */ > + rv = unlink("/dest/home/abc"); > + rtems_test_assert( rv == 0 ); > + > + /* Case 5: A non-empty directory exists where a file should be created */ > + rv = unlink("/dest/home/test_file"); > + rtems_test_assert( rv == 0 ); > + rv = mkdir("/dest/home/test_file", S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); > + rtems_test_assert( rv == 0 ); > + fd = creat("/dest/home/test_file/file", > + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > + rtems_test_assert( fd >= 0 ); > + close(fd); > + > + rv = Untar_FromFile( "/test.tar" ); > + printf("Untar from file; non-empty dir where file should be created - "); > + if (rv != UNTAR_FAIL) { > + printf ("error: untar didn't fail like expected: %i\n", rv); > + exit(1); > + } > + printf ("expected fail\n"); > + /* cleanup so that the next one works */ > + rv = unlink("/dest/home/test_file/file"); > + rtems_test_assert( rv == 0 ); > + rv = unlink("/dest/home/test_file"); > + rtems_test_assert( rv == 0 ); > + > + /* Case 6: A file exists where a directory is explicitly in the archive */ > + rv = unlink("/dest/home/dir/file"); > + rtems_test_assert( rv == 0 ); > + rv = unlink("/dest/home/dir"); > + rtems_test_assert( rv == 0 ); > + fd = creat("/dest/home/dir", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > + rtems_test_assert( fd >= 0 ); > + close(fd); > + > + rv = Untar_FromFile( "/test.tar" ); > + printf("Untar from file; overwrite file with explicit directory - "); > + if (rv != UNTAR_SUCCESSFUL) { > + printf ("error: untar failed: %i\n", rv); > + exit(1); > + } > + assert_content_like_expected(); > printf ("successful\n"); > > /******************/ > diff --git a/testsuites/libtests/tar01/tar01.doc > b/testsuites/libtests/tar01/tar01.doc > index 060f98a..adffdca 100644 > --- a/testsuites/libtests/tar01/tar01.doc > +++ b/testsuites/libtests/tar01/tar01.doc > @@ -20,3 +20,4 @@ directives: > concepts: > > + exercise these routines > ++ check whether existing files are overwritten or not overwritten like > expected > diff --git a/testsuites/libtests/tar01/tar01.scn > b/testsuites/libtests/tar01/tar01.scn > index 68fa951..dd72f95 100644 > --- a/testsuites/libtests/tar01/tar01.scn > +++ b/testsuites/libtests/tar01/tar01.scn > @@ -1,9 +1,24 @@ > -*** TAR01 TEST *** > -Untaring from memory - successful > +*** BEGIN OF TEST TAR 1 *** > +*** TEST VERSION: 6.0.0.e1efb4eb8a9d6dd5f6f37dafc9feb0a9e6a888f1 > +*** TEST STATE: EXPECTED_PASS > +*** TEST BUILD: RTEMS_POSIX_API > +*** TEST TOOLS: 10.3.1 20210409 (RTEMS 6, RSB > ad54d1dd3cf8249d9d39deb1dd28b2f294df062d-modified, Newlib eb03ac1) > +Untaring from memory - untar: memory at 0x11ece8 (10240) > +untar: symlink: home/test_file -> symlink > +untar: file: home/test_file (s:73,m:0644) > +untar: file: home/abc/def/test_script (s:21,m:0755) > +untar: dir: home/dir > +untar: file: home/dir/file (s:12,m:0644) > +successful > ========= /home/test_file ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > +========= /home/abc/def/test_script ========= > +(0)#! joel > +ls -las /dev > + > + /home/abc/def/test_script: mode: 0755 want: 0755 > ========= /symlink ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > @@ -11,35 +26,58 @@ initial tar image. > > Copy tar image to test.tar > Untaring from file - successful > +Untar from file into existing structure with one missing file - successful > +Untar from file; overwrite empty directory with file - successful > +Untar from file; file exists where parent dir should be created - expected > fail > +Untar from file; non-empty dir where file should be created - expected fail > +Untar from file; overwrite file with explicit directory - successful > ========= /dest/home/test_file ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > +========= /dest/home/abc/def/test_script ========= > +(0)#! joel > +ls -las /dev > + > + /dest/home/abc/def/test_script: mode: 0755 want: 0755 > ========= /dest/symlink ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > > -Untaring chunks from memory - untar: dir: home > -untar: file: home/test_file (73) > +Untaring chunks from memory - untar: symlink: home/test_file -> symlink > +untar: file: home/test_file (s:73,m:0644) > +untar: file: home/abc/def/test_script (s:21,m:0755) > +untar: dir: home/dir > +untar: file: home/dir/file (s:12,m:0644) > successful > ========= /dest2/home/test_file ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > +========= /dest2/home/abc/def/test_script ========= > +(0)#! joel > +ls -las /dev > + > + /dest2/home/abc/def/test_script: mode: 0755 want: 0755 > ========= /dest2/symlink ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > > -Untaring chunks from tgz- untar: dir: home > -untar: file: home/test_file (73) > -successful > +Untaring chunks from tgz - successful > ========= /dest3/home/test_file ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > > +========= /dest3/home/abc/def/test_script ========= > +(0)#! joel > +ls -las /dev > + > + /dest3/home/abc/def/test_script: mode: 0755 want: 0755 > ========= /dest3/symlink ========= > (0)This is a test of loading an RTEMS filesystem from an > initial tar image. > -*** END OF TAR01 TEST *** > + > + > +*** END OF TEST TAR 1 *** > diff --git a/testsuites/libtests/tar01/tar01.tar > b/testsuites/libtests/tar01/tar01.tar > index 6c6952e..9874f42 100644 > Binary files a/testsuites/libtests/tar01/tar01.tar and > b/testsuites/libtests/tar01/tar01.tar differ > > _______________________________________________ > vc mailing list > v...@rtems.org > http://lists.rtems.org/mailman/listinfo/vc > _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
