Skip to content

test commit for High level issues #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Sid2001-maker wants to merge 1 commit into from
Closed

test commit for High level issues #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/IJavaScriptContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
class IJavaScriptContext
{
public:
virtual ~IJavaScriptContext() = default;
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title "test commit don't merge" indicates this PR is not intended for merging. This appears to be a test or work-in-progress PR that should be closed or converted to draft.

Copilot uses AI. Check for mistakes.
virtual bool runScript(const char *script, bool isModule=true, std::string name="", const char *args = nullptr, bool isApplication=false) = 0;
virtual bool runFile(const char *file, const char* args, bool isApplication=false) = 0;
virtual std::string getUrl() = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/JSRuntimeServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class JsonWrap

uint32_t getUint32(const char *name, bool &err)
{
uint32_t res;
uint32_t res = 0;
cJSON *itm = cJSON_GetObjectItem(mPtr, name);
if (!itm || !cJSON_IsNumber(itm))
{
Expand Down
3 changes: 2 additions & 1 deletion src/NativeJSRenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,9 @@ void NativeJSRenderer::run()
if(!mTestFileName.empty())
{
ModuleSettings settings;
uint32_t id = createApplicationIdentifier();
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable shadowing detected: the local variable 'id' declared on line 509 shadows the outer scope variable 'id' declared at line 471. This can lead to confusion and potential bugs. Consider renaming one of these variables to avoid shadowing.

Copilot uses AI. Check for mistakes.
settings.enableJSDOM = mEnableTestFileDOMSupport;
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation: This line now uses spaces while the surrounding lines (510, 512) use tabs. The indentation should be consistent throughout the file. The original version used tabs, which was consistent with the surrounding code.

Suggested change
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);

Copilot uses AI. Check for mistakes.
NativeJSRenderer::createApplicationInternal(appRequest);
NativeJSRenderer::runApplicationInternal(appRequest);
mTestFileName = "";
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/JavaScriptContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ if (mModuleSettings.enablePlayer)
mPriv->releaseAllProtected();
JSGlobalContextRelease(mContext);
JSContextGroupRelease(mContextGroup);
if (mNetworkMetricsData) {
delete mNetworkMetricsData;
mNetworkMetricsData = nullptr;
}
rtLogInfo("%s end", __FUNCTION__);
}

Expand Down
2 changes: 2 additions & 0 deletions src/jsc/JavaScriptUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ rtError rtReadBinaryBinding(int numArgs, const rtValue* args, rtValue* result, v
{
result->setString(buffer);
}
free(buffer);
return RT_OK;
Comment on lines +335 to 336
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The free(buffer) call will not be reached if result is null and the early return path at line 336 is taken. Additionally, there are multiple error conditions (fopen failure, stat failure, malloc failure, fread failure) that are not checked, which could lead to undefined behavior or crashes. The function needs comprehensive error handling with proper cleanup on all paths.

Copilot uses AI. Check for mistakes.
Comment on lines +335 to 336
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function parameters (numArgs, args, context) are declared but never used. This suggests the function is incomplete or the parameters should be used for validation and file path specification. Consider adding UNUSED_PARAM macros for genuinely unused parameters or implementing proper parameter handling.

Copilot uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -693,6 +694,7 @@ rtError rtJSRuntimeDownloadMetrics(int numArgs, const rtValue* args, rtValue* re
rtValue keys;
if (map->Get("allKeys", &keys) != RT_OK) {
rtLogWarn("Could not retrieve url for network metrics data.");
delete netMetricsArray;
return RT_FAIL;
Comment on lines +697 to 698
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak: netMetricsArray is allocated before this point (line 692) but may be leaked if subsequent error paths at lines 704-706 or 718-720 return early without cleanup. While this fix addresses one leak on line 697, there are other early return paths (lines 704-706 and 718-720) that still need similar cleanup to avoid memory leaks.

Copilot uses AI. Check for mistakes.
}
rtObjectRef objRef = keys.toObject();
Expand Down
Loading