On Mon, Jun 16, 2008 at 04:38:03AM -0400, Jeff Johnson wrote: > Alexey: > This is your patch reworked slightly. See what you think.
> > + if (S_ISREG(flp->fl_mode)) {
> > + int bingo = 1;
> > + /* Hard links need be tallied only once. */
> > + if (flp->fl_nlink > 1) {
> > + FileListRec jlp = flp + 1;
> > + int j = i + 1;
> > + for (; (unsigned)j < fi->fc; j++, jlp++) {
Loop post-increment "j++, jlp++" is not enough here.
Remember that "jlp" can go ahead of "j" when folding dups.
You've got to rewind dups here the same way it is done in the outer loop.
while (...)
jlp++;
Here is a counterexample:
%install
mkdir -p %buildroot/foo
head -c 1024 /dev/zero >%buildroot/foo/1
head -c 1024 /dev/zero >%buildroot/foo/2
ln %buildroot/foo/1 %buildroot/foo/3
%files
/foo/1
/foo/2
/foo/2
/foo/3
We expect RPMTAG_SIZE == 2048 (1024 per /foo/1+3 hardlink set
plus 1024 per /foo/2). However, we get 3072. It is easy to
see why: when doing /foo/1, /foo/2 gets checked twice, and /foo/3
is not checked at all (and /foo/1 gets bingo but it shouldn't).
> > + if (!S_ISREG(jlp->fl_mode))
> > + continue;
> > + if (flp->fl_nlink != jlp->fl_nlink)
> > + continue;
> > + if (flp->fl_ino != jlp->fl_ino)
> > + continue;
> > + if (flp->fl_dev != jlp->fl_dev)
> > + continue;
Now assume that rewind went well, and we are at the last dup
entry (which has valid file flags that's been merged). You still
have to check:
if (jlp->fl_flags & (RPMFILE_EXCLUDE | RPMFILE_GHOST))
continue;
Think of this example:
%install
mkdir -p %buildroot/foo
head -c 1024 /dev/zero >%buildroot/foo/1
ln %buildroot/foo/1 %buildroot/foo/2
%files
/foo/1
%ghost /foo/2
> > + bingo = 0; /* don't tally hardlink yet. */
> > + break;
> > + }
> > + }
> > + if (bingo)
> > + fl->totalFileSize += flp->fl_size;
> > + }
pgp6bhd4q61NY.pgp
Description: PGP signature
