fix: read Molfile input from non-seekable files like /dev/stdin#238
Merged
Conversation
DetectInputINChIFileType() peeked the first lines of a named input file to recognise a plain-InChI file, then "rewound" by closing and reopening the file by path. Reopening only restores the read position for a regular file; for a non-seekable input (pipe, FIFO, /dev/stdin, process substitution) the consumed Molfile header was lost and the parser started mid-record, aborting with "Cannot interpret atom block line". Skip auto-detection when the stream is not seekable (it cannot be peeked non-destructively) and keep the declared MOLFILE/SDFILE type; a plain InChI can still be forced from a pipe with -InpAux. Rewind seekable input with fseek() instead of fclose()+fopen(), which also removes a latent NULL-dereference if the reopen ever failed. Add a POSIX regression test feeding a Molfile via /dev/stdin.
Unit Test Coverage ReportCoverage Regression Summary
Find details on the base coverage at https://iupac-inchi.github.io/InChI/coverage/index.html Find details on this PR's coverage by downloading coverage-reports-28949615220 and opening html/index.html |
There was a problem hiding this comment.
Pull request overview
This PR fixes a long-standing input-type auto-detection bug where DetectInputINChIFileType() destructively “peeked” non-seekable named inputs (e.g., /dev/stdin, FIFOs, process substitution) and then attempted to “rewind” by reopening via path, causing Molfile parsing to start mid-record.
Changes:
- Skip plain-InChI auto-detection when the input stream is not seekable; keep the user-declared MOLFILE/SDFILE type.
- Rewind seekable inputs via
fseek(…, 0, SEEK_SET)instead offclose()+fopen(). - Add a POSIX-only regression test that exercises
/dev/stdinas a named, non-seekable input.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| INCHI-1-SRC/INCHI_BASE/src/ichiparm.c | Avoids destructive peeking on non-seekable named inputs and rewinds seekable streams safely with fseek. |
| INCHI-1-TEST/tests/test_executable/test_named_pipe_input.py | Adds a regression test to ensure /dev/stdin works as a named input for Molfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JanCBrammer
approved these changes
Jul 6, 2026
JanCBrammer
left a comment
JanCBrammer
left a comment
Collaborator
There was a problem hiding this comment.
Not much to add to the CoPilot review. Maybe this would be a good opportunity to add a Doxygen comment to DetectInputINChIFileType?
… test Address review feedback on the regression test: also assert a zero exit code (so it fails if the executable prints the key but still errors), and store the molfile as a triple-quoted multi-line string to match the existing test-suite convention instead of one long escaped line. Also add a Doxygen comment to DetectInputINChIFileType describing the plain-InChI detection and why it is skipped for non-seekable input.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A follow-up to #236 / #237. While fixing the
-STDIOpipe regression I found a separate, pre-existing bug: passing a non-seekable named input —/dev/stdin, a FIFO, or a<(process substitution)— fails with:This is exactly the error @JervenBolleman hit when trying the
/dev/stdinworkaround in #236.Root cause
DetectInputINChIFileType()(inichiparm.c) auto-detects plain-InChI vs Molfile input by peeking the first 4 lines, then "rewinds" by closing and reopening the file by path:Reopening resets a regular file to the start, but a non-seekable stream cannot be rewound: the consumed Molfile header (name, comment, counts line) is gone, and reopening
/dev/stdinjust yields a fresh fd on the same, already-advanced pipe. The parser then begins mid-record and aborts. (This path only runs for a named input file, so realstdin— which skips detection — was unaffected.)The old code also never checked the reopen for
NULL, so a failedfopenwould leave a dangling*inp_file.Fix
fseek(*inp_file, 0, SEEK_SET)fails) — it can't be peeked non-destructively — and keep the declaredMOLFILE/SDFILEtype.fseek(0)instead offclose()+fopen(), removing the latent NULL-dereference.Behavior:
inchi-1 /dev/stdin …,<(…), FIFOs → now produce the correct InChI/InChIKey ✓-InpAuxforces it explicitly — verified working.Tests
test_named_pipe_input.pyfeeds the reported molecule via/dev/stdin. It fails against the pre-fix binary (reproduces the exact error) and passes against the fix.test_executablesuite green; the changed file is clean under-Wall -Wextra.Related: #236, #237.