Skip to content
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

p4est_save_ext fails on Windows due to different behavior of fopen/ftell #113

Closed
sloede opened this issue May 28, 2021 · 2 comments · Fixed by #114
Closed

p4est_save_ext fails on Windows due to different behavior of fopen/ftell #113

sloede opened this issue May 28, 2021 · 2 comments · Fixed by #114

Comments

@sloede
Copy link
Contributor

sloede commented May 28, 2021

Description
The function p4est_save_ext fails under Windows at the following line:

p4est/src/p4est.c

Line 3527 in 3a78fd2

SC_CHECK_ABORT (fpos > 0, "first file tell");

A similar issue occurs in p6est_save_ext at
SC_CHECK_ABORT (fpos > 0, "first file tell");

I manually instrumented the code at the problematic locations to show the return value of ftell, which gives the number of bytes from the beginning of the file to the file position indicator (see, e.g., here). The return value is indeed 0, indicating an empty file (an actual error would have been indicated by a return value of -1). However, by looking at the output file generated until this point, it is clear that the file is indeed non-empty. So what happened?

It took me a while to figure out the reason for this, but it is related due to the different behavior of ftell on Windows vs. Linux/macOS:

A few lines before the line in which the error occurs, the output file is opened in "(binary) append" mode using fopen(filename, "ab"):

p4est/src/p4est.c

Lines 3521 to 3527 in 3a78fd2

/* open file after writing connectivity to it */
file = fopen (filename, "ab");
SC_CHECK_ABORT (file != NULL, "file open");
/* align the start of the header */
fpos = ftell (file);
SC_CHECK_ABORT (fpos > 0, "first file tell");

According to most sources documenting the C function fopen (e.g., here or here), opening a file in "append" mode seems to mean that the file position indicator is set to the end of the (possibly existing) file to ensure that new data is written at the end. This behavior is used in p4est.c#L3527 and p6est.c#L761 to ensure that the previous write operation has succeeded by asserting the return value of ftell to be greater than zero.

However, on Windows the behavior of fopen/ftell differs in a small, but in this case significant, detail (source):

If no I/O operation has yet occurred on a file opened for appending, the file position is the beginning of the file.

and

When a file is opened for appending, the file position is moved to end of file before any write operation.

This causes the checks in p4est.c#L3527 and p6est.c#L761 to fail, since the return value of ftell after opening is 0, even though the file has been created and written to correctly before.

To Reproduce
An MWE to produce this error is, e.g., as follows:

#include <p4est.h>

int main(int argc, char **argv)
{
  p4est_t            *p4est;
  p4est_connectivity_t *conn;

  conn = p4est_connectivity_new_periodic();
  p4est = p4est_new(0, conn, 0, NULL, NULL);
  p4est_save("testfile", p4est, 0);

  /* Destroy the p4est and the connectivity structure. */
  p4est_destroy (p4est);
  p4est_connectivity_destroy (conn);

  return 0;
}

Compiling and running this under Windows yields the following output:

Into p4est_new with min quadrants 0 level 0 uniform 1
New p4est with 1 trees on 1 processors
Initial level 0 potential global quadrants 1 per tree 1
Done p4est_new with 1 total quadrants
Into p4est_save testfile
Abort: first file tell
Abort: src/p4est.c:3527
Abort

Additional information
More information on how to reproduce the erroneous behavior can be found at https://github.com/sloede/p4est-demo. It was reproduced under Windows using a current MSYS2 installation and the mingw-w64-x86_64-toolchain package.

@cburstedde
Copy link
Owner

Nice story, and thanks for the investigation!

@sloede
Copy link
Contributor Author

sloede commented Jun 1, 2021

Haha, sorry if I went a little overboard with the description, but since this was a real tricky one for me to figure out, I figured I should write it down to remind me of the solution should I ever have to debug something similar again in the future :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants