Fix OBJ reader using j as both triangle counter and vertex index

[sbryngelson]

Summary

Severity: CRITICAL — triangle assignments go to wrong indices; potential OOB crash.

File: src/common/m_model.fpp, lines 278-282

In the OBJ reader, j serves double duty as both the sequential triangle counter (initialized to 1 on line 264) and the third vertex index read from the face line. When read(line(3:), *) k, l, j executes, j is overwritten with the vertex index from the file, then immediately used as the triangle index in model%trs(j).

Before

j = 1  ! triangle counter
...
case ("f ")
    read (line(3:), *) k, l, j         ! j overwritten with vertex index
    model%trs(j)%v(1, :) = vertices(k, :)  ! j is now a vertex index, not triangle counter
    model%trs(j)%v(2, :) = vertices(l, :)
    model%trs(j)%v(3, :) = vertices(j, :)  ! also wrong: self-referential
    j = j + 1                           ! incrementing from wrong base

After

integer :: i, j, k, l, iv3, ...   ! new variable iv3
...
case ("f ")
    read (line(3:), *) k, l, iv3       ! third vertex index in separate var
    model%trs(j)%v(1, :) = vertices(k, :)  ! j is the triangle counter
    model%trs(j)%v(2, :) = vertices(l, :)
    model%trs(j)%v(3, :) = vertices(iv3, :)
    j = j + 1

Why this went undetected

Only triggers when loading OBJ STL models, a specialized feature with limited test coverage.

Test plan

• Load an OBJ model file and verify triangle geometry matches the file

🤖 Generated with Claude Code

Fixes #1199

[codeant-ai]

CodeAnt AI is reviewing your PR.

---

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

[coderabbitai]

Warning

Rate limit exceeded

@sbryngelson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 28 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codeant-ai]

Nitpicks 🔍

🔒 No security issues identified

⚡ Recommended areas for review Missing bounds checks The code now assigns vertex coordinates using indices read from the file directly into model%trs(j)%v(...). There are no checks that k, l, iv3 are in 1..nVertices, or that j is in 1..model%ntrs, which can still cause out-of-bounds accesses for malformed or unexpected OBJ files. OBJ face-token formats The new read assumes face indices are simple whitespace-separated integers. Many OBJ files use 'v/vt/vn' (slashes) or negative indices; the current read will fail or produce wrong indices for those cases. Consider normalizing tokens (replace '/' with ' ') or explicitly parse tokens to extract vertex indices before internal read.

[codeant-ai]

CodeAnt AI finished reviewing your PR.

[Copilot]

Pull request overview

This PR fixes a critical bug in the OBJ file reader where the variable j was incorrectly used for both tracking the triangle counter and storing the third vertex index from face definitions. This caused triangle vertex assignments to be written to incorrect array indices and could result in out-of-bounds memory access.

Changes:

• Introduced a new variable iv3 to store the third vertex index read from OBJ face lines
• Updated the face parsing logic to use iv3 instead of reusing j

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.05%. Comparing base (84c46e0) to head (651c50b).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/common/m_model.fpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1178   +/-   ##
=======================================
  Coverage   44.05%   44.05%           
=======================================
  Files          70       70           
  Lines       20498    20498           
  Branches     1990     1990           
=======================================
  Hits         9030     9030           
  Misses      10329    10329           
  Partials     1139     1139           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sbryngelson]

Superseded by #1240 (batched safe fixes)