This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, fortran] PR22539 implement FSEEK intrinsic


Daniel Franke wrote:
I updated the patch according to comments of Brooks Moses and Jerry DeLisle.

Due to Brooks concers, I was also able to identify a possible problem in the function fd_seek (libgfortran/io/unix.c), where the physical and logical offsets of the internal representation of the file were changed, regardless of the status of lseek. The patch now include a fix for this as well. The testcase was updated to include a seek with a resulting negative offset.

Ah, good -- when I was glancing through fd_seek, that did look a little suspicious; I'm glad not to have to worry about whether I should have looked at it closer. Thank you! :)


Jerry asked:
I have not seen what happens if we try to seek a read only file past its
current end. That would be worth checking.

Nothing much. The position is moved beyond the end. Attempting to read there results in a "Fortran runtime error: End of file". Moving beyond the end and back works as expected, i.e. read works without problems.

Good. That sounds like appropriate behavior.


With all the changes in place and no regressions on i686-pc-linux-gnu, ok for trunk?

I do have one other question, which I apologize for neglecting to mention earlier: you mentioned, at some point, handling various kinds of the STATUS variable by some form of conversion, if I'm remembering correctly. I don't see any of that in the patch, though, and it looks like you're constructing a call that passes along the STATUS argument with whatever kind it happens to be, and then fseek_sub expects an int*.


If you want to just document that the STATUS argument "shall be a scalar of type @code{INTEGER(4)}, and add the following check that kind(status)==4 in gfc_check_fseek_sub, then I think that's acceptable:

  if (kind_value_check(status, 0, 4) == FAILURE)
    return FAILURE;

(Note that this should be literally 4, not gfc_default_integer_kind; sometimes the default integer kind is 8, and your code doesn't support that.)


I'll preapprove the non-libgfortran changes with that correction, but I don't know the library code well enough to feel confident approving things there. Jerry?


- Brooks


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]