[tlbuild] dvi test?
John Hawkinson
jhawk at alum.mit.edu
Tue Feb 1 22:12:25 CET 2022
Replies in-line to myself (from yesterday quoting Karl) as well as Bruno H.:
John Hawkinson <jhawk at alum.mit.edu> wrote on Mon, 31 Jan 2022
at 21:42:19 EST in <YfieC5DQs05suCNm at louder-room.local>:
> Karl Berry <karl at freefriends.org> wrote on Mon, 31 Jan 2022
> at 21:25:22 EST in <202202010225.2112PM9E003560 at freefriends.org>:
>
> > I think it would be a good idea to check the return values from those
> > fseeks.
>
> Yes, especially since the system call trace has it returning EINVAL (22).
>
...
> Anyhow, sure enough:
...
> - fseek(target_fp, 0L, SEEK_SET);
> - fseek(source_fp, 0L, SEEK_SET);
> + if (fseek(target_fp, 0L, SEEK_SET)) {
> + perror("fseek of target_fp");
...
> produces (with my other debugging interspersed):
>
> 3debug try_fopen /Users/jhawk/src/xdvik-22.87.05/Work/texk/xdvik/t2a.dvi for fd 4
> fseek of target_fp: Invalid argument
...
> of course, we need to do more than just print diagnostics, we should presumably return NULL which will stop the crash, but doesn't quite tell us what went wrong. But definitely good hints at where to look.
Right, so, here's a patch that checks all these system call returns.
It, umm, uses goto to remove the massive code duplication, and also uses a fallthrough because sometimes there is something to fclose() and sometimes there's not, and fclose(NULL) is not allowed. I don't feel so hot about this code, but it produces the proper result on the t2a test case when hitting shift-R:
jhawk at lrr xdvik % TEXMFROOT=/usr/local/texlive/2021 TEXMFCNF=$TEXMFROOT/texmf-dist/web2c ./xdvi-bin t2a.dvi
xdvi-bin: Error: Couldn't seek to start of file /var/folders/m2/16zlwjgj2p5d3qdy99z2gzf40000gn/T/xdvi-DpeSZS: Invalid argument - disabling `useTempFp'; target_fp: 0x219c06d40.
jhawk at lrr xdvik %
That is, it logs an error, disables the temporary file copying code, and moves on. Patch here:
diff --git a/texk/xdvik/dvi-init.c b/texk/xdvik/dvi-init.c
index 6126a08..7c664c2 100644
--- a/texk/xdvik/dvi-init.c
+++ b/texk/xdvik/dvi-init.c
@@ -1170,17 +1170,13 @@ make_backup_fp(FILE *source_fp, FILE *target_fp)
if (first_time) { /* doesn't exist yet, create it */
if ((tmp_fd = xdvi_temp_fd(&m_tmp_dvi_name)) == -1) {
XDVI_ERROR((stderr, "error creating temporary file - disabling `useTempFp'."));
- resource.use_temp_fp = False;
- remove_tmp_dvi_file(NULL);
- return NULL;
+ goto fail;
}
/* fprintf(stderr, "temporary file name: |%s|, %d\n", m_tmp_dvi_name, tmp_fd); */
TRACE_EVENTS((stderr, "Created temp file: |%s|\n", m_tmp_dvi_name));
if ((target_fp = try_fdopen(tmp_fd, "wb+")) == NULL) {
XDVI_ERROR((stderr, "error opening temporary file (%s) - disabling `useTempFp'.", strerror(errno)));
- resource.use_temp_fp = False;
- remove_tmp_dvi_file(NULL);
- return NULL;
+ goto fail;
}
first_time = False;
}
@@ -1189,19 +1185,22 @@ make_backup_fp(FILE *source_fp, FILE *target_fp)
ASSERT(target_fp != NULL, "");
ASSERT(source_fp != NULL, "");
+ if (fseek(target_fp, 0L, SEEK_SET)) {
+ XDVI_ERROR((stderr, "Couldn't seek to start of file %s: %s - disabling `useTempFp'; target_fp: %p.",
+ m_tmp_dvi_name, strerror(errno), target_fp));
+ goto fclose_and_fail;
+ }
#if HAVE_FTRUNCATE
if (ftruncate(tmp_fd, 0) < 0) {
-
XDVI_ERROR((stderr, "Couldn't truncate file %s: %s - disabling `useTempFp'; target_fp: %p.",
m_tmp_dvi_name, strerror(errno), target_fp));
- resource.use_temp_fp = False;
- remove_tmp_dvi_file(NULL);
- fclose(target_fp);
- return NULL;
+ goto fclose_and_fail;
}
#endif
- fseek(target_fp, 0L, SEEK_SET);
- fseek(source_fp, 0L, SEEK_SET);
+ if (fseek(source_fp, 0L, SEEK_SET)) {
+ perror("fseek of source_fp");
+ goto fclose_and_fail;
+ }
}
/* copy the file */
@@ -1210,22 +1209,32 @@ make_backup_fp(FILE *source_fp, FILE *target_fp)
"Error creating temporary file: %s\n"
"- disabling `useTempFp'.",
strerror(errno)));
- remove_tmp_dvi_file(NULL);
- resource.use_temp_fp = False;
- fclose(target_fp);
- target_fp = NULL;
+ goto fclose_and_fail;
}
/* rewind both files, else DVI parsing will fail! */
if (target_fp != NULL) {
fflush(target_fp);
}
- fseek(source_fp, 0L, SEEK_SET);
+ if (fseek(source_fp, 0L, SEEK_SET)) {
+ perror("fseek of source_fp after rewind");
+ }
if (target_fp != NULL) {
- fseek(target_fp, 0L, SEEK_SET);
+ if (fseek(target_fp, 0L, SEEK_SET)) {
+ perror("fseek of target_fp again");
+ }
}
return target_fp;
+
+ fclose_and_fail:
+ fclose(target_fp);
+ /* FALLTHROUGH */
+ fail:
+ resource.use_temp_fp = False;
+ remove_tmp_dvi_file(NULL);
+ return NULL;
+
}
static Boolean
Bruno Haible <bruno at clisp.org> wrote on Tue, 1 Feb 2022
at 14:20:43 EST in <6328302.7s5MMGUR32 at omega>:
> > > > 3208/0x31ed: ftruncate(0x6, 0x0, 0x0) = 0 0
> > > > 3208/0x31ed: lseek(0x6, 0xFFFFFFFFFFFFF7C7, 0x1) = -1 Err#22
> > ...
> > Note that in this case, it's not ftruncate() that's failing but rather fseek()/lseek(),
>
> This explains now why it's OS dependent: The code is shrinking a file
> to length 0, and then asking to move the file descriptor's position
> within that file backwards by 2105 bytes.
Well.
Funny you put it that way. I'm not sure the "code" is doing that,
where "the code" is xdvi (as opposed to the OS). Because the code was (minus error-checking):
ftruncate(tmp_fd, 0) < 0)
fseek(target_fp, 0L, SEEK_SET);
fseek(source_fp, 0L, SEEK_SET);
So again we have to wonder how our SEEK_SET was becoming a negative SEEK_CUR.
Also, the ftruncate(2) manpage under OSX does say:
Note: ftruncate() and truncate() do not modify the current file offset for any open file descriptions associated with the file.
> How about doing things in the opposite order? I.e. move the fseek()
> call, that calls lseek(), _before_ ftruncate. Then it should behave well,
> independently of the operating system.
But in any event, reordering them to put the ftruncate() after 1 or both fseek()s does not solve the problem. (err, the patch is now stupid long):
diff --git a/texk/xdvik/dvi-init.c b/texk/xdvik/dvi-init.c
index cc07714..3c84de5 100644
--- a/texk/xdvik/dvi-init.c
+++ b/texk/xdvik/dvi-init.c
@@ -1185,13 +1185,6 @@ make_backup_fp(FILE *source_fp, FILE *target_fp)
ASSERT(target_fp != NULL, "");
ASSERT(source_fp != NULL, "");
-#if HAVE_FTRUNCATE
- if (ftruncate(tmp_fd, 0) < 0) {
- XDVI_ERROR((stderr, "Couldn't truncate file %s: %s - disabling `useTempFp'; target_fp: %p.",
- m_tmp_dvi_name, strerror(errno), target_fp));
- goto fclose_and_fail;
- }
-#endif
if (fseek(target_fp, 0L, SEEK_SET)) {
XDVI_ERROR((stderr, "Couldn't seek to start of file %s: %s - disabling `useTempFp'; target_fp: %p.",
m_tmp_dvi_name, strerror(errno), target_fp));
@@ -1201,6 +1194,13 @@ make_backup_fp(FILE *source_fp, FILE *target_fp)
perror("fseek of source_fp");
goto fclose_and_fail;
}
+#if HAVE_FTRUNCATE
+ if (ftruncate(tmp_fd, 0) < 0) {
+ XDVI_ERROR((stderr, "Couldn't truncate file %s: %s - disabling `useTempFp'; target_fp: %p.",
+ m_tmp_dvi_name, strerror(errno), target_fp));
+ goto fclose_and_fail;
+ }
+#endif
}
/* copy the file */
Still gives me:
jhawk at lrr xdvik % TEXMFROOT=/usr/local/texlive/2021 TEXMFCNF=$TEXMFROOT/texmf-dist/web2c ./xdvi-bin t2a.dvi
xdvi-bin: Error: Couldn't seek to start of file /var/folders/m2/16zlwjgj2p5d3qdy99z2gzf40000gn/T/xdvi-i8KKvP: Invalid argument - disabling `useTempFp'; target_fp: 0x219c06d40.
which is to say it still fails but now that there's error-handling it no longer leads to a crash.
Great thought though, I was sure it was gonna work...until it didn't :(
--
jhawk at alum.mit.edu
John Hawkinson
More information about the tlbuild
mailing list.