440771 - (InspectTool) [meta] A few source code bugs identified with 'inspect' from the AT&T Toolchest

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[niteowltx]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.14) Gecko/20080418 Ubuntu/7.10 (gutsy) Firefox/2.0.0.14
Build Identifier: 

Here are a few source code bugs in firefox 3.0.  I downloaded:
        http://releases.mozilla.org/pub/mozilla.org/firefox/releases/3.0/source/firefox-3.0-source.tar.bz2
which was dated '17-Jun-2008 02:35'.  I have a very old code inspection tool
that is good at finding typical C coding errors.  After filtering out the false positives,
I was left with these items.

Extra semicolon:
security/nss/cmd/pk11mode/pk11mode.c:5018               if (kmo.hClientKey != CK_INVALID_HANDLE);
uriloader/exthandler/os2/nsOSHelperAppService.cpp:1464  if ((pos = fileType.Find("%S")) > -1);
widget/src/os2/nsRwsService.cpp:467                     if ((pos = classViewer.Find("%S")) > -1);
browser/components/shell/src/nsMacShellService.cpp:165  if (::CFURLGetFSRef(firefoxURL, &firefoxFSRef)); {
dbm/tests/lots.c:211                                    while(!(status = (database->seq) (database, &key, &data, R_NEXT)));

Dangling else:
intl/uconv/util/nsUCSupport.cpp:130                     } else for (;;) {

Using | instead of &:
accessible/src/html/nsHTMLSelectAccessible.cpp:338      if (*aState | nsIAccessibleStates::STATE_FOCUSED) {
build/wince/shunt/stdio.cpp:215                         if (mode | O_RDWR || (mode | O_WRONLY))  // write only == read|write
build/wince/shunt/stdio.cpp:217                         if (mode | O_CREAT)
build/wince/shunt/stdio.cpp:221                         else if (mode | O_APPEND)
build/wince/shunt/stdio.cpp:225                         else if (mode | O_TRUNC)
build/wince/shunt/stdio.cpp:235                         else if (mode | O_RDONLY)

== has precedence over |, need () or ???:
dom/src/base/nsJSEnvironment.cpp:3032                   action->sa_flags == SA_RESTART | SA_SIGINFO);

works by accident (!= has precedence over &):
xpcom/reflect/xptcall/src/md/unix/xptcstubs_nto_shle.cpp:83     if ( floatCount & 1 != 0 ) {

probably okay, but confusing:
toolkit/components/places/src/nsNavHistoryAutoComplete.cpp:356  if (moreChunksToSearch = mPreviousChunkOffset != -1)
extensions/spellcheck/src/mozInlineSpellWordUtil.cpp:594        for (PRInt32 i = node == mSoftEnd.mNode ? mSoftEnd.mOffset : 0;


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Jesse Ruderman]

Nice finds!  We'll need a separate bug for each file in order to get patches reviewed by the right people and checked in.  Want me to do that for you?

I'm curious which tool you used and how many false positives you had to wade through.

Comment 2

[niteowltx]

Sure, break it up however you like.

The tool is an  ancient program called 'inspect' which was part of what was
called the AT&T Toolchest.  It has problems mostly with not understanding
the difference between unary and binary &.  For this run, I counted 9349
files (*.c, *.h and *.cpp only) for which inspect reported 3859 items.  Culling
the list resulted in the items shown.  Inspect doesn't like some code which
is mostly a style issue. Code like:
             a = b == c;
drives it nuts but is really just a programmer style issue.

Comment 3

[Jesse Ruderman]

I don't see a "dangling else" ambiguity at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/intl/uconv/util/nsUCSupport.cpp&rev=1.12&mark=130#113 .  Am I missing something?

Comment 4

[Jesse Ruderman]

I filed bugs on each problem, except for the one in comment 3 and the "probably okay, but confusing" section.  I attached patches for the "extra semicolon" bugs, but not for the others, I just CCed the appropriate developers.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:overholt, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Andrew McCreight [:mccr8]]

This is a meta bug where all of the dependent bugs have been fixed.