From cfa1d7c54cc7afc6ce911d3cfaf459780f72c8ca Mon Sep 17 00:00:00 2001 From: Matthew Flatt Date: Wed, 21 Jun 2017 16:30:44 -0600 Subject: [PATCH] add more information to `copy-file` error message The `file/cache` relies on matching the text of a message, and the text had changed to have less information on Unix, including whether the problem was with the source or destination file. Add a notion of error step to rktio and use it to improve the `copy-file` message. --- racket/src/racket/src/file.c | 28 +++++++++++++++++++++++++++- racket/src/rktio/rktio.h | 27 +++++++++++++++++++++++---- racket/src/rktio/rktio_error.c | 10 ++++++++++ racket/src/rktio/rktio_fs.c | 23 +++++++++++++++++++---- racket/src/rktio/rktio_private.h | 1 + 5 files changed, 80 insertions(+), 9 deletions(-) diff --git a/racket/src/racket/src/file.c b/racket/src/racket/src/file.c index 1492e7e727..299c76e3f2 100644 --- a/racket/src/racket/src/file.c +++ b/racket/src/racket/src/file.c @@ -3477,11 +3477,37 @@ static Scheme_Object *copy_file(int argc, Scheme_Object **argv) filename_for_error(argv[0]), filename_for_error(argv[1])); } else { + const char *how; + + switch (rktio_get_last_error_step(scheme_rktio)) { + case RKTIO_COPY_STEP_OPEN_SRC: + how = "cannot open source file"; + break; + case RKTIO_COPY_STEP_OPEN_DEST: + how = "cannot open destination file"; + break; + case RKTIO_COPY_STEP_READ_SRC_DATA: + how = "error reading source file"; + break; + case RKTIO_COPY_STEP_WRITE_DEST_DATA: + how = "error writing destination file"; + break; + case RKTIO_COPY_STEP_READ_SRC_METADATA: + how = "error reading source-file metadata"; + break; + case RKTIO_COPY_STEP_WRITE_DEST_METADATA: + how = "error writing destination-file metadata"; + break; + default: + how = "copy failed"; + } + scheme_raise_exn(MZEXN_FAIL_FILESYSTEM, - "copy-file: copy failed\n" + "copy-file: %s\n" " source path: %q\n" " destination path: %q\n" " system error: %R", + how, filename_for_error(argv[0]), filename_for_error(argv[1])); } diff --git a/racket/src/rktio/rktio.h b/racket/src/rktio/rktio.h index 7d8686fb09..9c95b9dc23 100644 --- a/racket/src/rktio/rktio.h +++ b/racket/src/rktio/rktio.h @@ -779,24 +779,38 @@ RKTIO_EXTERN rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char rktio_bool_t exists_ok); /* Starts a file copy. Depending on the OS, this step may perform the whole copy, or it may just get started. Can report - `RKTIO_ERROR_EXISTS`. */ + `RKTIO_ERROR_EXISTS`, and sets an error step as listed further below. */ RKTIO_EXTERN rktio_bool_t rktio_copy_file_is_done(rktio_t *rktio, rktio_file_copy_t *fc); RKTIO_EXTERN rktio_ok_t rktio_copy_file_step(rktio_t *rktio, rktio_file_copy_t *fc); /* As long as the copy isn't done, call `rktio_copy_file_step` to make - a little progress. Use `rktio_copy_file_finish_permissions` (optionally) - and then `rktio_copy_file_stop` when done. */ + a little progress. Use `rktio_copy_file_finish_permissions` + (optionally) and then `rktio_copy_file_stop` when done. An error + sets an error step as listed further below. */ RKTIO_EXTERN rktio_ok_t rktio_copy_file_finish_permissions(rktio_t *rktio, rktio_file_copy_t *fc); /* Depending on the OS, copies permissions from the source to the destination. This step can be performed at any time between the start and stop. Reports success if this step isn't needed (e.g., - where a copy fully completes when it is started). */ + where a copy fully completes when it is started). On error, the + step is set to `RKTIO_COPY_STEP_WRITE_DEST_METADATA`. */ RKTIO_EXTERN void rktio_copy_file_stop(rktio_t *rktio, rktio_file_copy_t *fc); /* Deallocates the copy process, interrupting it if the copy is not complete. */ +/* Step values for errors from `rktio_copy_file_start` and + `rktio_copy_file_step`: */ +enum { + RKTIO_COPY_STEP_UNKNOWN, + RKTIO_COPY_STEP_OPEN_SRC, + RKTIO_COPY_STEP_OPEN_DEST, + RKTIO_COPY_STEP_READ_SRC_DATA, + RKTIO_COPY_STEP_WRITE_DEST_DATA, + RKTIO_COPY_STEP_READ_SRC_METADATA, + RKTIO_COPY_STEP_WRITE_DEST_METADATA +}; + /*************************************************/ /* System paths */ @@ -1059,7 +1073,12 @@ enum { RKTIO_ERROR_CONVERT_OTHER, }; +RKTIO_EXTERN int rktio_get_last_error_step(rktio_t *rktio); +/* Some operations report further information about the step that + failed. The meaning of a step number is operation-specific. */ + RKTIO_EXTERN void rktio_set_last_error(rktio_t *rktio, int kind, int errid); +RKTIO_EXTERN void rktio_set_last_error_step(rktio_t *rktio, int step); /* In case you need to save and restore error information. */ RKTIO_EXTERN void rktio_remap_last_error(rktio_t *rktio); diff --git a/racket/src/rktio/rktio_error.c b/racket/src/rktio/rktio_error.c index a71c080287..5f77b8b12f 100644 --- a/racket/src/rktio/rktio_error.c +++ b/racket/src/rktio/rktio_error.c @@ -78,12 +78,22 @@ int rktio_get_last_error_kind(rktio_t *rktio) return rktio->errkind; } +int rktio_get_last_error_step(rktio_t *rktio) +{ + return rktio->errstep; +} + void rktio_set_last_error(rktio_t *rktio, int kind, int errid) { rktio->errkind = kind; rktio->errid = errid; } +void rktio_set_last_error_step(rktio_t *rktio, int new_errstep) +{ + rktio->errstep = new_errstep; +} + void rktio_remap_last_error(rktio_t *rktio) { if (rktio->errkind == RKTIO_ERROR_KIND_RACKET) { diff --git a/racket/src/rktio/rktio_fs.c b/racket/src/rktio/rktio_fs.c index 5409a5a359..0b4a234284 100644 --- a/racket/src/rktio/rktio_fs.c +++ b/racket/src/rktio/rktio_fs.c @@ -1589,8 +1589,10 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const rktio_fd_t *src_fd, *dest_fd; src_fd = rktio_open(rktio, src, RKTIO_OPEN_READ); - if (!src_fd) + if (!src_fd) { + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_OPEN_SRC); return NULL; + } do { ok = fstat(rktio_fd_system_fd(rktio, src_fd), &buf); @@ -1601,6 +1603,7 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const get_posix_error(); else set_racket_error(RKTIO_ERROR_IS_A_DIRECTORY); + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_READ_SRC_METADATA); rktio_close(rktio, src_fd); return NULL; } @@ -1609,6 +1612,7 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const | (exists_ok ? RKTIO_OPEN_TRUNCATE : 0))); if (!dest_fd) { rktio_close(rktio, src_fd); + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_OPEN_DEST); return NULL; } @@ -1632,9 +1636,15 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const const wchar_t *dest_w; src_w = WIDE_PATH_copy(src); - if (!src_w) return NULL; + if (!src_w) { + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_OPEN_SRC); + return NULL; + } dest_w = WIDE_PATH_temp(dest); - if (!dest_w) return NULL; + if (!dest_w) { + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_OPEN_DEST); + return NULL; + } if (CopyFileW(src_w, dest_w, !exists_ok)) { rktio_file_copy_t *fc; @@ -1651,6 +1661,7 @@ rktio_file_copy_t *rktio_copy_file_start(rktio_t *rktio, const char *dest, const set_racket_error(RKTIO_ERROR_EXISTS); else get_windows_error(); + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_UNKNOWN); free(src_w); @@ -1677,14 +1688,17 @@ rktio_ok_t rktio_copy_file_step(rktio_t *rktio, rktio_file_copy_t *fc) fc->done = 1; return 1; } else if (len == RKTIO_READ_ERROR) { + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_READ_SRC_DATA); return 0; } else { intptr_t done = 0, amt; while (done < len) { amt = rktio_write(rktio, fc->dest_fd, buffer + done, len - done); - if (amt < 0) + if (amt < 0) { + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_WRITE_DEST_DATA); return 0; + } done += amt; } return 1; @@ -1706,6 +1720,7 @@ rktio_ok_t rktio_copy_file_finish_permissions(rktio_t *rktio, rktio_file_copy_t if (err) { get_posix_error(); + rktio_set_last_error_step(rktio, RKTIO_COPY_STEP_WRITE_DEST_METADATA); return 0; } #endif diff --git a/racket/src/rktio/rktio_private.h b/racket/src/rktio/rktio_private.h index f2798ab7a3..a6394e4739 100644 --- a/racket/src/rktio/rktio_private.h +++ b/racket/src/rktio/rktio_private.h @@ -33,6 +33,7 @@ struct rktio_t { int errid; int errkind; + int errstep; #ifdef RKTIO_SYSTEM_WINDOWS char *last_err_str; #endif