Closed Bug 536352 Opened 10 years ago Closed 10 years ago

missed null check and memory leak

Categories

(Firefox Build System :: General, defect)

All
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ekrem.tomur, Assigned: ekrem.tomur)

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.16) Gecko/2009121601 Ubuntu/8.10 (intrepid) Firefox/3.0.16 GTB6
Build Identifier: 

in code of latest trunk http://hg.mozilla.org/mozilla-central:

in file config/nsinstall.c 264:
dir should be null checked because opendir function may return null.

in the same function destdir should be freed before return.

Reproducible: Always
Attached patch proposed patch to fix (obsolete) — Splinter Review
proposed patch to fix the null check and memory leak issue
Version: unspecified → Trunk
Component: General → Build Config
QA Contact: general → build-config
Attachment #418837 - Flags: review?(ted.mielczarek)
Comment on attachment 418837 [details] [diff] [review]
proposed patch to fix

>diff -r 1f0e04dc2b21 config/nsinstall.c
>--- a/config/nsinstall.c	Tue Dec 22 09:47:18 2009 +0000
>+++ b/config/nsinstall.c	Tue Dec 22 13:07:58 2009 +0200
>@@ -256,17 +256,21 @@ copydir( char *from, char *to, mode_t mo
>   /* create destination directory */
>   destdir = xmalloc((unsigned int)(strlen(to) + 1 + strlen(base) + 1));
>   sprintf(destdir, "%s%s%s", to, _DIRECTORY_SEPARATOR, base);
>   if (mkdirs(destdir, mode) != 0) {
>     fail("cannot make directory %s\n", destdir);
>     return;

You should probably put a free(destdir) in here for consistency.

Otherwise looks fine. If you attach a patch with that change I'll land it for you.
Attachment #418837 - Flags: review?(ted.mielczarek) → review+
Assignee: nobody → ekrem.tomur
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
patch updated as required, thanks for reviewing
Attachment #418837 - Attachment is obsolete: true
Attachment #418982 - Flags: review?(ted.mielczarek)
Attachment #418982 - Flags: review?(ted.mielczarek) → review+
http://hg.mozilla.org/mozilla-central/rev/1aa561085c31
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Hardware: x86 → All
Resolution: --- → FIXED
I forgot to sync with js/src/config initially:
http://hg.mozilla.org/mozilla-central/rev/586eea23fcef
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.