Skip to content

File postions in boss/logex/fmpsee limited to 31 bits #115

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.

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

Open
6 tasks
wehimwich opened this issue Apr 26, 2021 · 1 comment
Open
6 tasks

File postions in boss/logex/fmpsee limited to 31 bits #115

wehimwich opened this issue Apr 26, 2021 · 1 comment

Comments

@wehimwich
Copy link
Member

The variables that handle file positions in fmpsee/ library routines fmpsetpos() and fmpposition() are int but should be long instead. They were changed in the bulk unlongify of c9cbc93 and never changed back.

Fortunately, this is benign as long as the files are less than 231 bytes long. Since boss only uses file positions for .snp and .prc files, this is unlikely to create a problem, even on 64-bit systems. It seems likely that this breaks at least some parts of logex; It also seems likely that no one uses it anymore (let us know if you do). These reasons, and the amount of work to fix the calling FORTRAN, probably made this clean-up a low priority task. In fact, switching to int variables could be viewed as a win in the short-term. That may have been the actual decision at the time, but if it was (things were busy), I don't remember it. Anyway, the same limitation existed in the previous 32-bit only FS versions.

In boss, the FORTRAN generally has the affected variables declared integer*4. To extend the range for 64-bit systems, these variables will probably need to be changed into two element arrays. Some of these values are also stored in other integer arrays. Some adjustment will be needed for that. It seem unlikely any of these variables (or array elements) are used in arithmetic but that should be checked for carefully as well.

In logex, at least some of the involved variables are declared integer*2, so it seems likely that some commands, probably at least plot, won't work, even on 32-bit systems. It seems unlikely anyone is still using logex, but it may not take much work to clean this up.

In the same commit, the line number variables in the fmpsetline() routine were also changed to int. Whether .snp files with 231 or more lines need to be supported is a different question. If so, it would have ramifications in the line number handling in boss as well. This appears to be an issue for logex for processing .log files.

Pieces needed to support larger .snp and .prc files:

  • Fix fmpsetpos() and fmpposition()
  • Expand size of affected FORTRAN variables
  • Accommodate larger variables when stored in FORTRAN arrays
  • Check for arithmetic with FORTRAN file positions variables/arrays

Line numbers in .skd files:

  • Consider/fix how many lines should be allowed in .snp files (fmpsetline() and more generally in boss).

logex:

  • Consider fixing logex
@dehorsley
Copy link
Member

Good catch. I think some of these incorrect ints could be caught by the compiler (with -Wconversion I think), but I seem to remember there was too many false positives.

wehimwich added a commit that referenced this issue Jun 25, 2021
Was changed by bulk unlongify in c9cbc93, but not fixed after.

There are still other files tha need to be fixed, see #115.
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

No branches or pull requests

2 participants