Closed
Bug 53244
Opened 24 years ago
Closed 23 years ago
[RFE] Recursive directory creation in Unix installer
Categories
(SeaMonkey :: Installer, enhancement, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.0
People
(Reporter: agracebush, Assigned: slogan)
References
Details
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
slogan
:
review+
dveditz
:
superreview+
|
Details | Diff | Splinter Review |
Installer does not allow 2 (or more) levels of directory creation during
installation process
Linux installer, unlike Win Installer, only allows user to enter one level of
directory creation. ie you can select /u/user/test2/test1 (assuming
/u/user/exists) - installer will prompt to create but will only create test2 and
not next level test1
Comment 2•24 years ago
|
||
This is by design, i.e., non-recursive dir creation. Grace, does the installer
display a different dir than is created (e.g., does it display
/u/user/test2/test1/ when in fact it only created /u/user/test2 or does the
display not lie)?
At any rate, we can fix this to do recursive dir creation; next release though.
Status: NEW → ASSIGNED
Summary: Installer does not allow 2 (or more) levels of directory creation during installation process → [RFE] Recursive directory creation in Unix installer
Target Milestone: --- → Future
Updated•24 years ago
|
QA Contact: gemal → gbush
Reporter | ||
Comment 3•24 years ago
|
||
Installer displays path I set, asks me if I want to create, when I say yes- it
gives me error (-624)(only problem is it does not remove this dialog)
It does not allow me to install so I change to one level of creation and I am
able to install successfully.
Comment 4•24 years ago
|
||
I see the problem. When you click OK on the error dialog it doesn't go away but
the OK button disappears. This is poor UE that should get fixed next release too.
Updated•24 years ago
|
Priority: P3 → P1
Comment 6•23 years ago
|
||
nominating this simple change
sounds like fun
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.6
Comment 8•23 years ago
|
||
Comment 9•23 years ago
|
||
Something like attachment 54052 [details] [diff] [review] ?
Assignee | ||
Comment 10•23 years ago
|
||
+ memcpy(path, gCtx->opt->mDestination, (pathLen > PATH_MAX) ? PATH_MAX :
pathLen);
+ path[pathLen] = '/'; // for uniform handling
So, you copy PATHMAX if pathLen is greater than PATHMAX. At first glance, that
appears ok. But, let pathLen == PATHMAX * 2. You will then be setting
path[PATHMAX * 2] to '/', exceeding the array bounds because you are using
pathLen in that second statement.
How about :
if ( pathLen > MAXPATH)
pathLen = MAXPATH;
memcpy( path, gCtx->opt->mDestination, pathLen );
path[pathLen] = '/';
path[pathLen + 1] = '\0';
this is simpler, eliminates the potential buffer overflow. Note also I added the
NULL ('\0') that you forgot to add to the end of the buffer (can't assume the
memory is filled with NULLs).
I notice you check for errs but do nothing with them. What happens if you fail a
mkdir?
Comment 11•23 years ago
|
||
> You will then be setting path[PATHMAX * 2] to '/', exceeding
> the array bounds because you are using pathLen in that second
> statement.
Oops, sorry, missed that
> path[pathLen + 1] = '\0';
> NULL ('\0') that you forgot to add to the end of the buffer (can't assume the
> memory is filled with NULLs).
Actually, I don't need terminate array with '\0', because I'll replace '/' with
'\0' inside the loop and I don't rely on '\0' in loop condition
( for (int i = 1; !err && i <= pathLen; i++) )
> I notice you check for errs but do nothing with them. What happens if you fail a
> mkdir?
Those lines didn't fit into diff, look a copule of lines below:
if (err != 0)
{
ErrorHandler(E_MKDIR_FAIL);
return;
}
Comment 12•23 years ago
|
||
Comment 13•23 years ago
|
||
Assignee | ||
Comment 14•23 years ago
|
||
Comment on attachment 56242 [details] [diff] [review]
fix case typos in prev patch
looks ok, thanks for the patch
Attachment #56242 -
Flags: review+
Keywords: mozilla0.9.6
Comment 15•23 years ago
|
||
Comment on attachment 56242 [details] [diff] [review]
fix case typos in prev patch
sr=dveditz
Attachment #56242 -
Flags: superreview+
Keywords: nsbeta1+
Target Milestone: mozilla0.9.8 → mozilla1.0
Assignee | ||
Comment 16•23 years ago
|
||
fixed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 18•22 years ago
|
||
[RFE] is deprecated in favor of severity: enhancement. They have the same meaning.
Severity: normal → enhancement
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•