I've looked at the situation here and I think I agree with Egmont's
analysis in his original report in November 2004.  In particular, I
agree with his assertion about the memory leak due to failed lstats in
the code in 1.10.24:

    cfile->namenode->filestat = (struct stat *) nfmalloc(sizeof(struct stat));
    if (lstat(cfile->namenode->name, cfile->namenode->filestat)) {
      cfile->namenode->filestat= 0;

However, in current code we have this:

        if (!cfile->namenode->filestat) {
          cfile->namenode->filestat= nfmalloc(sizeof(struct stat));
          if (lstat(cfile->namenode->name, cfile->namenode->filestat)) {
            ...
            memset(cfile->namenode->filestat, 0,
                   sizeof(cfile->namenode->filestat));

which doesn't leak the memory.  It zeroes the contents of the
allocated struct, thus preventing the same struct being allocated
again later.  (However, the sizeof is wrong.)  This is I think correct
and it also avoids the 5500^2 lstat calls, so the current code ought
to be much better in both ways.

This was a change I submitted as part of the fix for #108587 and was
committed by Frank Lichtenheld in February 2006 (commit ref 854ad168)
and released in 1.13.14.

Therefore think Egmont's updated part1 patch against 1.13.22 in his
message in June 2006 is incorrect.  One effect of that updated part1
patch is that the 5500^2 lstats will come back.

I haven't done any testing of these conclusions.

Egmont, can you confirm whether you were still seeing the memory
exhaustion and excessive lstats when you prepared your patch against
1.13.22, or did you just work textually to fix up the edit conflicts ?


Regarding the second part of Egmont's report, that dpkg is statting
the wrong file, he is of course completely correct.  His part2 patch
is correct IMO.

I have resolved the conflicts resulting from applying part2 of his
patch without part1 to a current version, and fixed the incorrect use
of sizeof for memset, and the result is here:
  http://git.debian.org/?p=users/iwj/dpkg.git
  git://git.debian.org/git/users/iwj/dpkg.git
etc. as the branch
  bug281057

For reference, Egmont's part1 patch is there as `bug281057koblinger1'
but that change should IMO not be merged.

Ian.




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to