On Mon, Jan 18, 2010 at 02:06:07PM -0800, Carl Miller wrote:
> On Mon, Jan 18, 2010 at 11:52:12AM -0800, Carl Miller wrote:
> >
> > So now my question is, why do callers of add_inode() not check the
> > st_nlinks for being > 1 first, like callers of add_link_defer()? Is
> > there some reason that it *should* be this way? If not, I'd propose
> > putting that check in before calling add_inode(), and only having files
> > with nlinks >= 2 in the inode hash table. Thoughts?
> >
> > (Proposed patch to follow while others think of reasons this might break
> > something else...)
>
> It looks to me like copypass.c, the other user of add_inode() only adds
> to the hash table if st_nlinks > 1. That eases my concerns about having
> copyout.c do the same. Here's my proposed patch....
>
>
> c...@mithrandir:/tmp/cpio_patch/cpio-2.10$ diff -du src/copyout.c{.orig,}
> --- src/copyout.c.orig 2009-02-14 10:15:50.000000000 -0800
> +++ src/copyout.c 2010-01-18 14:02:20.000000000 -0800
> @@ -232,7 +232,8 @@
> header->c_name);
> warn_if_file_changed(header->c_name, file_hdr.c_filesize,
> file_hdr.c_mtime);
>
> - if (archive_format == arf_tar || archive_format == arf_ustar)
> + if ((archive_format == arf_tar || archive_format == arf_ustar)
> + && (file_hdr.c_nlink > 1))
> add_inode (file_hdr.c_ino, file_hdr.c_name, file_hdr.c_dev_maj,
> file_hdr.c_dev_min);
>
> @@ -652,7 +653,7 @@
>
> if (archive_format == arf_tar || archive_format == arf_ustar)
> {
> - if (file_hdr.c_mode & CP_IFDIR)
> + if ((file_hdr.c_mode & CP_IFMT) == CP_IFDIR)
> {
> int len = strlen (input_name.ds_string);
> /* Make sure the name ends with a slash */
> @@ -696,7 +697,8 @@
> switch (file_hdr.c_mode & CP_IFMT)
> {
> case CP_IFREG:
> - if (archive_format == arf_tar || archive_format == arf_ustar)
> + if ((archive_format == arf_tar || archive_format == arf_ustar)
> + && (file_hdr.c_nlink > 1))
> {
> char *otherfile;
> if ((otherfile = find_inode_file (file_hdr.c_ino,
> @@ -743,7 +745,8 @@
> warn_if_file_changed(orig_file_name, file_hdr.c_filesize,
> file_hdr.c_mtime);
>
> - if (archive_format == arf_tar || archive_format == arf_ustar)
> + if ((archive_format == arf_tar || archive_format == arf_ustar)
> + && (file_hdr.c_nlink > 1))
> add_inode (file_hdr.c_ino, orig_file_name, file_hdr.c_dev_maj,
> file_hdr.c_dev_min);
>
> @@ -777,7 +780,7 @@
> orig_file_name);
> continue;
> }
> - else if (archive_format == arf_ustar)
> + else if ((archive_format == arf_ustar) && (file_hdr.c_nlink >
> 1))
> {
> char *otherfile;
> if ((otherfile = find_inode_file (file_hdr.c_ino,
> ----------------
>
> I'll compile it and run the same test case against it now...
r...@mithrandir:/tmp/unpack# mkdir mnt_228
r...@mithrandir:/tmp/unpack# mount -t cramfs -o ro,nodev,noexec,loop
/usr/local/isengard/share/builds/build228/disk_fs.img mnt_228
r...@mithrandir:/tmp/unpack# ls -l mnt_228/dev/loop?
brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 mnt_228/dev/loop0
brw-r--r-- 1 root root 7, 1 1969-12-31 16:00 mnt_228/dev/loop1
brw-r--r-- 1 root root 7, 2 1969-12-31 16:00 mnt_228/dev/loop2
brw-r--r-- 1 root root 7, 3 1969-12-31 16:00 mnt_228/dev/loop3
r...@mithrandir:/tmp/unpack# cd mnt_228
r...@mithrandir:/tmp/unpack/mnt_228# ( echo "dev" ; echo "dev/loop0" ; echo
"dev/loop1" ; echo "dev/loop2" ; echo "dev/loop3" ) |
/tmp/cpio_patch/cpio-2.10/src/cpio -o -F /tmp/looptest.tar -H ustar --quiet
r...@mithrandir:/tmp/unpack/mnt_228# tar -tf /tmp/looptest.tar
tar: Record size = 7 blocks
dev/
dev/loop0
dev/loop1
dev/loop2
dev/loop3
r...@mithrandir:/tmp/unpack/mnt_228# cd ..
r...@mithrandir:/tmp/unpack# tar -xvf /tmp/looptest.tar
tar: Record size = 7 blocks
dev/
dev/loop0
tar: dev/loop0: implausibly old time stamp 1969-12-31 16:00:00
dev/loop1
tar: dev/loop1: implausibly old time stamp 1969-12-31 16:00:00
dev/loop2
tar: dev/loop2: implausibly old time stamp 1969-12-31 16:00:00
dev/loop3
tar: dev/loop3: implausibly old time stamp 1969-12-31 16:00:00
tar: dev: implausibly old time stamp 1969-12-31 16:00:00
r...@mithrandir:/tmp/unpack# ls -l dev
total 0
brw-r--r-- 1 root root 7, 0 1969-12-31 16:00 loop0
brw-r--r-- 1 root root 7, 1 1969-12-31 16:00 loop1
brw-r--r-- 1 root root 7, 2 1969-12-31 16:00 loop2
brw-r--r-- 1 root root 7, 3 1969-12-31 16:00 loop3
r...@mithrandir:/tmp/unpack#
Much better. No trailing slashes; all archived files unpack, and have the
correct st_rdev. I'm going to call that a fix.
Thanks much, Clint and Tim!
-------Carl
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
