Skip to content

refactor: eliminate duplicate import_concoredocker.m logic and fix Windows-only batch generation (fixes #285)#397

Merged
pradeeban merged 1 commit intoControlCore-Project:devfrom
GaneshPatil7517:refactor/import-concore-docker-cleanup
Feb 19, 2026
Merged

refactor: eliminate duplicate import_concoredocker.m logic and fix Windows-only batch generation (fixes #285)#397
pradeeban merged 1 commit intoControlCore-Project:devfrom
GaneshPatil7517:refactor/import-concore-docker-cleanup

Conversation

@GaneshPatil7517
Copy link
Copy Markdown
Contributor

@GaneshPatil7517 GaneshPatil7517 commented Feb 18, 2026

Hi @pradeeban Sir
This PR resolves Issue #285 by removing duplication between import_concore.m and import_concoredocker.m, and addresses Copilot review feedback on file descriptor leaks and error handling.

Changes Made

  • import_concore.m: Wrapped concorekill.bat generation in an ispc guard so it only executes on Windows
  • import_concore.m: Added fopen return value check (~= -1) before writing concorekill.bat, with a graceful warning on failure
  • import_concore.m: Added proper fclose for concore.iport and concore.oport file descriptors in both success and error paths
  • import_concoredocker.m: Removed concorekill.bat generation entirely (Docker containers always run Linux)
  • import_concoredocker.m: Replaced try/catch with fopen return value check and guaranteed fclose for concore.iport / concore.oport

Behavior

Environment concorekill.bat created?
Windows (non-Docker) Yes
Linux (non-Docker) No
Docker (Linux) No

Scope

  • Limited to import_concore.m and import_concoredocker.m
  • No changes to path detection logic
  • No changes to Verilog, Python, or other language files
  • No functional changes for Windows users

Testing

  • All 77 existing tests pass
  • Verified Windows batch file is generated only when ispc is true
  • Verified Docker variant contains no .bat generation code
  • File descriptors are now properly closed on all code paths
image

Copilot AI review requested due to automatic review settings February 18, 2026 06:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses issue #285 by preventing Windows-only concorekill.bat generation in non-Windows environments, especially for Docker-generated (Linux) builds of the MATLAB/Octave runtime helpers.

Changes:

  • Wrapped concorekill.bat generation in import_concore.m with an ispc guard so it only runs on Windows.
  • Removed concorekill.bat generation from import_concoredocker.m (Docker/Linux template).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
import_concore.m Generates concorekill.bat only on Windows via ispc guard; rest of initialization unchanged.
import_concoredocker.m Removes Windows batch generation from the Docker template while keeping the same init logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… generation in Docker, and fix file descriptor leaks (fixes ControlCore-Project#285)
@GaneshPatil7517 GaneshPatil7517 force-pushed the refactor/import-concore-docker-cleanup branch from 9891ba1 to 4dd4d7d Compare February 18, 2026 06:41
@pradeeban
Copy link
Copy Markdown
Member

/gemini-review

@pradeeban pradeeban merged commit 95c337d into ControlCore-Project:dev Feb 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants