Skip to content
Snippets Groups Projects
Commit a398abb2 authored by Rob Swindell's avatar Rob Swindell :speech_balloon:
Browse files

Don't heap allocate argument to MsgBase and FileBase constructors

Nelgin reported a weird error with a failed very large allocation for the
base/code argument to the FileBase constructor. There's no good reason
these strings were heap-allocated in the first place, so just change to
use a stack allocated variable instead. I don't know why this would fix
anything, but at least there's one less heap allocation and potential
for memory leak here.

Fix 2 bugs in js_update_file():
1. missing parenthesis (really?!? Caught by Coverity - sigh) in last
commmit caused attempt/failure/error to remove file after making any
updates. The removing is only supposed to happen when its necessary to
remove and re-add the file to the filebase (e.g. updating extended
description or auxdata).
2. Wrong filename used in 'removing' exception string.
parent 09d721d4
No related branches found
No related tags found
No related merge requests found
...@@ -1330,18 +1330,18 @@ js_update_file(JSContext *cx, uintN argc, jsval *arglist) ...@@ -1330,18 +1330,18 @@ js_update_file(JSContext *cx, uintN argc, jsval *arglist)
if(file.extdesc != NULL) if(file.extdesc != NULL)
truncsp(file.extdesc); truncsp(file.extdesc);
if(!readd_always && strcmp(extdesc ? extdesc : "", file.extdesc ? file.extdesc : "") == 0 if(!readd_always && strcmp(extdesc ? extdesc : "", file.extdesc ? file.extdesc : "") == 0
&& strcmp(auxdata ? auxdata : "", file.auxdata ? file.auxdata : "") == 0) && strcmp(auxdata ? auxdata : "", file.auxdata ? file.auxdata : "") == 0) {
p->smb_result = smb_putfile(&p->smb, &file); p->smb_result = smb_putfile(&p->smb, &file);
if(p->smb_result != SMB_SUCCESS) if(p->smb_result != SMB_SUCCESS)
JS_ReportError(cx, "%d writing '%s'", p->smb_result, file.name); JS_ReportError(cx, "%d writing '%s'", p->smb_result, file.name);
else { } else {
if((p->smb_result = smb_removefile_by_name(&p->smb, filename)) == SMB_SUCCESS) { if((p->smb_result = smb_removefile_by_name(&p->smb, filename)) == SMB_SUCCESS) {
if(readd_always) if(readd_always)
file.hdr.when_imported.time = 0; // we want the file to appear as "new" file.hdr.when_imported.time = 0; // we want the file to appear as "new"
p->smb_result = smb_addfile(&p->smb, &file, SMB_SELFPACK, extdesc, auxdata, newfname); p->smb_result = smb_addfile(&p->smb, &file, SMB_SELFPACK, extdesc, auxdata, newfname);
} }
else else
JS_ReportError(cx, "%d removing '%s'", p->smb_result, file.name); JS_ReportError(cx, "%d removing '%s'", p->smb_result, filename);
} }
} }
} }
...@@ -1811,7 +1811,7 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -1811,7 +1811,7 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
{ {
JSObject * obj; JSObject * obj;
jsval * argv=JS_ARGV(cx, arglist); jsval * argv=JS_ARGV(cx, arglist);
char* base = NULL; char base[MAX_PATH + 1] = "";
private_t* p; private_t* p;
scfg_t* scfg; scfg_t* scfg;
...@@ -1827,15 +1827,13 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -1827,15 +1827,13 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
memset(p,0,sizeof(private_t)); memset(p,0,sizeof(private_t));
p->smb.retry_time=scfg->smb_retry_time; p->smb.retry_time=scfg->smb_retry_time;
JSSTRING_TO_MSTRING(cx, JS_ValueToString(cx, argv[0]), base, NULL); JSVALUE_TO_STRBUF(cx, argv[0], base, sizeof base, NULL);
if(JS_IsExceptionPending(cx)) { if(JS_IsExceptionPending(cx)) {
if(base != NULL)
free(base);
free(p); free(p);
return JS_FALSE; return JS_FALSE;
} }
if(base==NULL) { if(*base=='\0') {
JS_ReportError(cx, "Invalid base parameter"); JS_ReportError(cx, "Invalid 'code' parameter");
free(p); free(p);
return JS_FALSE; return JS_FALSE;
} }
...@@ -1843,7 +1841,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -1843,7 +1841,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
if(!JS_SetPrivate(cx, obj, p)) { if(!JS_SetPrivate(cx, obj, p)) {
JS_ReportError(cx,"JS_SetPrivate failed"); JS_ReportError(cx,"JS_SetPrivate failed");
free(p); free(p);
free(base);
return JS_FALSE; return JS_FALSE;
} }
...@@ -1864,7 +1861,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -1864,7 +1861,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
SAFECOPY(p->smb.file, base); SAFECOPY(p->smb.file, base);
} }
free(base);
return JS_TRUE; return JS_TRUE;
} }
......
...@@ -3350,9 +3350,8 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -3350,9 +3350,8 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
{ {
JSObject * obj; JSObject * obj;
jsval * argv=JS_ARGV(cx, arglist); jsval * argv=JS_ARGV(cx, arglist);
JSString* js_str;
JSObject* cfgobj; JSObject* cfgobj;
char* base = NULL; char base[MAX_PATH + 1] = "";
private_t* p; private_t* p;
scfg_t* scfg; scfg_t* scfg;
...@@ -3368,16 +3367,13 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -3368,16 +3367,13 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
memset(p,0,sizeof(private_t)); memset(p,0,sizeof(private_t));
p->smb.retry_time=scfg->smb_retry_time; p->smb.retry_time=scfg->smb_retry_time;
js_str = JS_ValueToString(cx, argv[0]); JSVALUE_TO_STRBUF(cx, argv[0], base, sizeof base, NULL);
JSSTRING_TO_MSTRING(cx, js_str, base, NULL);
if(JS_IsExceptionPending(cx)) { if(JS_IsExceptionPending(cx)) {
if(base != NULL)
free(base);
free(p); free(p);
return JS_FALSE; return JS_FALSE;
} }
if(base==NULL) { if(*base=='\0') {
JS_ReportError(cx, "Invalid base parameter"); JS_ReportError(cx, "Invalid 'code' parameter");
free(p); free(p);
return JS_FALSE; return JS_FALSE;
} }
...@@ -3387,7 +3383,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -3387,7 +3383,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
if(!JS_SetPrivate(cx, obj, p)) { if(!JS_SetPrivate(cx, obj, p)) {
JS_ReportError(cx,"JS_SetPrivate failed"); JS_ReportError(cx,"JS_SetPrivate failed");
free(p); free(p);
free(base);
return JS_FALSE; return JS_FALSE;
} }
...@@ -3441,7 +3436,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist) ...@@ -3441,7 +3436,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
} }
} }
free(base);
return JS_TRUE; return JS_TRUE;
} }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment