Closed Bug 325471 Opened 18 years ago Closed 18 years ago

Broken search for path to binary on linux in xulrunner-stub

Categories

(Toolkit Graveyard :: XULRunner, defect)

Product:
Toolkit Graveyard
Component:
XULRunner
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: bugz, Assigned: bugz)

Details

(Keywords: fixed1.8.0.4, fixed1.8.1)

Attachments

(1 file, 2 obsolete files)

v3
2.44 KB, patch
benjamin
: first-review+
benjamin
: approval-branch-1.8.1+
dveditz
: approval1.8.0.2-
jay
: approval1.8.0.4+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060131 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060131 Firefox/1.6a1

Current method of searching for real path to xulrunner-stub in somewhat broken. First it can lead to infinite loop, and also doesn't resolve symlinks for paths from PATH.

I attach patch shortly.

Reproducible: Always

Steps to Reproduce:
Attached patch v1 (obsolete) — — Splinter Review 
This fixes infinite loop, and add symlinks resolving to xulrunner-stub
Attachment #210366 - Flags: first-review?(benjamin)
Comment on attachment 210366 [details] [diff] [review]
v1

Could you provide a patch that does the same thing for the apprunner?

http://lxr.mozilla.org/mozilla/source/toolkit/xre/nsAppRunner.cpp#1122

The tokenizing there is correct, but it should probably use realpath as you do here.
Attached patch v2 (obsolete) — — Splinter Review 
This patches also nsAppRunner.cpp
Attachment #210366 - Attachment is obsolete: true
Attachment #210371 - Flags: first-review?(benjamin)
Attachment #210366 - Flags: first-review?(benjamin)
Comment on attachment 210371 [details] [diff] [review]
v2

>     while ( (token = nsCRT::strtok(newStr, ":", &newStr)) ) {
>       sprintf(exePath, "%s/%s", token, argv0);
>-      if (stat(exePath, &fileStat) == 0) {
>+      if (realpath(tmpPath, exePath) && stat(exePath, &fileStat) == 0) {
>         found = PR_TRUE;
>         break;
>       }

You mean to change the sprintf to point to tmpPath, right?
Attachment #210371 - Flags: first-review?(benjamin) → first-review-
Attached patch v3 — — Splinter Review 
(In reply to comment #4)
> You mean to change the sprintf to point to tmpPath, right?
> 

Right.
Attachment #210371 - Attachment is obsolete: true
Attachment #210374 - Flags: first-review?(benjamin)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 210374 [details] [diff] [review]
v3

r=me, get somebody on irc.mozilla.org #developers to land this. I'll grant 1.8.1 once we verify this works on trunk: 1.8.0.2 depends on getting some decent QA for this.
Attachment #210374 - Flags: first-review?(benjamin)
Attachment #210374 - Flags: first-review+
Attachment #210374 - Flags: branch-1.8.1?
Attachment #210374 - Flags: approval1.8.0.2?
Assignee: nobody → prefiks
Landed by timeless on the trunk.
mozilla/toolkit/xre/nsAppRunner.cpp 	1.126 	mozilla/xulrunner/stub/nsXULStub.cpp 	1.5
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Attachment #210374 - Flags: approval1.8.0.2? → approval1.8.0.2-
Attachment #210374 - Flags: approval1.8.0.3?
Attachment #210374 - Flags: approval-branch-1.8.1?
Attachment #210374 - Flags: approval-branch-1.8.1+
This is a low-risk patch and I really want the xulrunner-stub bits for 1.8.0.3. The firefox-bin parts are not a big deal because we always launch firefox-bin with the shell script which passes the full path as argv[0].
Keywords: fixed1.8.1
Comment on attachment 210374 [details] [diff] [review]
v3

Please check in promptly on the 1.8.0 branch.  Thanks!
Attachment #210374 - Flags: approval1.8.0.3? → approval1.8.0.3+
Fixed on 1.8.0
Keywords: fixed1.8.0.3
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.