225604 - Dreftool should not follow symlinks (by default)

Status: RESOLVED FIXED

(Core Graveyard :: Tracking, defect)

Description

[timeless]

#0  0x4024edac in ios::eof (this=0xbfffdd18) at
../../../gcc-2.95.3/libio/streambuf.h:218
#1  0x08050de6 in CPattern::OutputMozillaFilename (this=0xbfffe504,
anOutput=@0x4025b080)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:552
#2  0x0804f50d in CPattern::scan (this=0xbfffe504, aTokenizer=@0xbfffe588,
anOutput=@0x4025b080)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:718
#3  0x0804f0bb in CPattern::scan (this=0xbfffe504, aTokenizer=@0xbfffe588,
anOutput=@0x4025b080)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:639
#4  0x0804e4ce in ScanFile (aFilename=@0xbfffe748, anOutput=@0x4025b080,
aLineCount=@0xbffff134,
    anErrCount=@0xbffff130) at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:753
#5  0x0804e7d9 in IteratePath (
    aPath=0x8207f20
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~/obj-i686-pc-linux-gnu-qt/gfx/src/gtk/nsFT2FontCatalog.cpp",
anOutput=@0x4025b080, aF
ilecount=@0xbffff138, aLinecount=@0xbffff134,
    anErrcount=@0xbffff130) at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:780
#6  0x0804e755 in IteratePath (
    aPath=0x8200c90
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~/obj-i686-pc-linux-gnu-qt/gfx/src/gtk",
anOutput=@0x4025b080, aFilecount=@0xbffff138,
 aLinecount=@0xbffff134,
    anErrcount=@0xbffff130) at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#7  0x0804e755 in IteratePath (
    aPath=0x82132a0
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~/obj-i686-pc-linux-gnu-qt/gfx/src",
anOutput=@0x4025b080, aFilecount=@0xbffff138, aLi
necount=@0xbffff134, anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#8  0x0804e755 in IteratePath (
    aPath=0x812f370
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~/obj-i686-pc-linux-gnu-qt/gfx",
anOutput=@0x4025b080, aFilecount=@0xbffff138, aLineco
unt=@0xbffff134, anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#9  0x0804e755 in IteratePath (
    aPath=0x8063690
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~/obj-i686-pc-linux-gnu-qt",
anOutput=@0x4025b080, aFilecount=@0xbffff138, aLinecount=
@0xbffff134, anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#10 0x0804e755 in IteratePath (
    aPath=0x8064750
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla/~",
    anOutput=@0x4025b080, aFilecount=@0xbffff138, aLinecount=@0xbffff134,
anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#11 0x0804e755 in IteratePath (
    aPath=0x805c288
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend/mozilla",
    anOutput=@0x4025b080, aFilecount=@0xbffff138, aLinecount=@0xbffff134,
anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#12 0x0804e755 in IteratePath (
    aPath=0xbfffef9c
"/mnt/ibm/mozhack/mozilla/tools/tinderbox/Linux_2.4.20_Depend",
anOutput=@0x4025b080,
    aFilecount=@0xbffff138, aLinecount=@0xbffff134, anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
---Type <return> to continue, or q <return> to quit---
#13 0x0804e755 in IteratePath (aPath=0xbffff09c
"/mnt/ibm/mozhack/mozilla/tools/tinderbox",
    anOutput=@0x4025b080, aFilecount=@0xbffff138, aLinecount=@0xbffff134,
anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#14 0x0804e755 in IteratePath (aPath=0xbffff34e
"/mnt/ibm/mozhack/mozilla/tools", anOutput=@0x4025b080,
    aFilecount=@0xbffff138, aLinecount=@0xbffff134, anErrcount=@0xbffff130)
    at /mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:772
#15 0x0804eb2c in main (argc=3, argv=0xbffff1a4) at
/mnt/ibm/mozhack/mozilla/tools/dreftool/main.cpp:843
#16 0x402a917d in __libc_start_main () from /lib/libc.so.6

I only wanted dreftool to analyze mozilla/tools, but mozilla/tools/tinderbox/
has symlinks to mozilla/ and to other things. The result wasn't very good.

Comment 1

[timeless]

Attachment: drop ostreams, switch from PRFile to nsIFile, check istreams for filenotfound

So, this stack demonstrated a few problems, one was recursion risk (symlinks),
another was that it was trying (forever) to read from a file that didn't exist.

Comment 2

[[e-mail]]

Comment on attachment 135672 [details] [diff] [review]
drop ostreams, switch from PRFile to nsIFile, check istreams for filenotfound

>Index: main.cpp
>===================================================================
>@@ -579,37 +585,36 @@
>-      if(gEmitHTML) {
>-        anOutput << "<br><a href=\"" ;
>-        if (found)
>-          anOutput
>-            << "http://bonsai.mozilla.org/cvsblame.cgi?file="
>-            << buf
>-            << "&rev="
>-            << version
>-            << "&mark="
>-            << mLineNumber
>-            << "#"
>-            << mLineNumber - 5;
>-        else
>-          anOutput
>-            << "http://lxr.mozilla.org/mozilla/source/"
>-            << buf
>-            << "#"
>-            << mLineNumber;
>-
>-        cout << "\">";  
>-      }
>-
>-      cout << buf << ":" << mLineNumber;
>-
>-      if(gEmitHTML) {
>-        cout << "</a>";
>-      }
>-
>+      if(gEmitHTML)
>+        fprintf(stdout, "<br><a href=\"");
>+      if (found)
>+        fprintf(stdout,
>+                "http://bonsai.mozilla.org/cvsblame.cgi?file=%s"
>+                "&rev=%s"
>+                "&mark=%d"
>+                "#%d",
>+                buf,
>+                version,
>+                mLineNumber,
>+                mLineNumber - 5);
>+      else if (gEmitHTML)
>+        fprintf(stdout,
>+                "http://lxr.mozilla.org/mozilla/source/%s#%d",
>+                buf,
>+                mLineNumber);
>+      else
>+        fprintf(stdout,
>+                "%s:%d",
>+                buf,
>+                mLineNumber);
>+      if (gEmitHTML)
>+        fprintf(stdout,
>+                "\">%s:%d</a>",
>+                buf,
>+                mLineNumber);
>     }

what's going on here? are you sure you meant to change this logic?

>@@ -718,11 +723,11 @@
>-                  anOutput << "   Deref-error: \""
>-                           << PromiseFlatCString(theString).get()
>-                           << "\"" << endl;
>+                  fprintf(stdout,
>+                          "   Deref-error: \"%s\"\n",
>+                          PromiseFlatCString(theString).get());

drop the |PromiseFlatCString|, it's unneeded

>+void IteratePath(nsIFile *aPath,
>+  int& aFilecount,int& aLinecount,int& anErrcount)
>+{
>+        nsCOMPtr<nsISupports> thing; nsCOMPtr<nsIFile> path;

declare on separate lines please.

>@@ -834,34 +845,46 @@
>+    else
>+      fprintf(stderr, "OOPS\n");

something more descriptive, perhaps?

r=dwitte

Comment 3

[timeless]

> what's going on here?
the non bonsai case ended up w/o that info. I only noticed it when
I ran dreftool on a non CVS file.
> are you sure you meant to change this logic?
yes

> drop the |PromiseFlatCString|, it's unneeded
good point

> declare on separate lines please.
done

> something more descriptive, perhaps?
done