From 1953c287ace801a6606abd6c3ead0ff345645bec Mon Sep 17 00:00:00 2001 From: Randy Palamar Date: Sun, 23 Nov 2025 19:09:25 -0700 Subject: text-io: make TextSave visible Having a failure case on allocing a TextSave is stupid. Ideally there would be no allocations in the file saving path but we have to replace the braindead dirname(3) with an internal implementation. --- configure | 3 +++ sam.c | 19 +++++++------- text-io.c | 88 ++++++++++++++++++++------------------------------------------- text.h | 15 +++++++++-- 4 files changed, 53 insertions(+), 72 deletions(-) diff --git a/configure b/configure index d90731f..a70eac1 100755 --- a/configure +++ b/configure @@ -236,6 +236,9 @@ esac tryflag CFLAGS -Wall tryflag CFLAGS -pipe +# shut up useless warning +tryflag CFLAGS -Wno-initializer-overrides + # Try flags to optimize binary size tryflag CFLAGS -O2 tryflag CFLAGS -ffunction-sections diff --git a/sam.c b/sam.c index e835367..379b674 100644 --- a/sam.c +++ b/sam.c @@ -1688,8 +1688,8 @@ static bool cmd_write(Vis *vis, Win *win, Command *cmd, const char *argv[], Sele if (write_entire_file) *r = text_range_new(0, text_size(text)); - TextSave *ctx = text_save_begin(text, AT_FDCWD, path, file->save_method); - if (!ctx) { + TextSave ctx = text_save_default(.txt = text, .method = file->save_method, .filename = path); + if (!text_save_begin(&ctx)) { const char *msg = errno ? strerror(errno) : "try changing `:set savemethod`"; vis_info_show(vis, "Can't write `%s': %s", path, msg); goto err; @@ -1700,18 +1700,19 @@ static bool cmd_write(Vis *vis, Win *win, Command *cmd, const char *argv[], Sele for (Selection *s = view_selections(&win->view); s; s = view_selections_next(s)) { Filerange range = visual ? view_selections_get(s) : *r; - ssize_t written = text_save_write_range(ctx, &range); + ssize_t written = text_save_write_range(&ctx, &range); failure = (written == -1 || (size_t)written != text_range_size(&range)); - if (failure) { - text_save_cancel(ctx); + if (failure || !visual) break; - } + } - if (!visual) - break; + if (!failure) { + failure = !text_save_commit(&ctx); + } else { + text_save_cancel(&ctx); } - if (failure || !text_save_commit(ctx)) { + if (failure) { vis_info_show(vis, "Can't write `%s': %s", path, strerror(errno)); goto err; } diff --git a/text-io.c b/text-io.c index bd5eee7..94654ae 100644 --- a/text-io.c +++ b/text-io.c @@ -19,14 +19,6 @@ #include "text-util.h" #include "util.h" -struct TextSave { /* used to hold context between text_save_{begin,commit} calls */ - Text *txt; /* text to operate on */ - char *filename; /* filename to save to as given to text_save_begin */ - char *tmpname; /* temporary name used for atomic rename(2) */ - int fd; /* file descriptor to write data to using text_save_write */ - int dirfd; /* directory file descriptor, relative to which we save */ - enum TextSaveMethod type; /* method used to save file */ -}; /* Allocate blocks holding the actual file content in chunks of size: */ #ifndef BLOCK_SIZE @@ -318,15 +310,11 @@ static bool text_save_begin_atomic(TextSave *ctx) { close(oldfd); } - ctx->type = TEXT_SAVE_ATOMIC; return true; err: saved_errno = errno; if (oldfd != -1) close(oldfd); - if (ctx->fd != -1) - close(ctx->fd); - ctx->fd = -1; errno = saved_errno; return false; } @@ -350,7 +338,7 @@ static bool text_save_commit_atomic(TextSave *ctx) { free(ctx->tmpname); ctx->tmpname = NULL; - int dir = openat(ctx->dirfd, dirname(ctx->filename), O_DIRECTORY|O_RDONLY); + int dir = openat(ctx->dirfd, dirname(ctx->tmpname), O_DIRECTORY|O_RDONLY); if (dir == -1) return false; @@ -405,15 +393,11 @@ static bool text_save_begin_inplace(TextSave *ctx) { * here we are screwed, TODO: make a backup before? */ if (ftruncate(ctx->fd, 0) == -1) goto err; - ctx->type = TEXT_SAVE_INPLACE; return true; err: saved_errno = errno; if (newfd != -1) close(newfd); - if (ctx->fd != -1) - close(ctx->fd); - ctx->fd = -1; errno = saved_errno; return false; } @@ -430,63 +414,45 @@ static bool text_save_commit_inplace(TextSave *ctx) { return true; } -TextSave *text_save_begin(Text *txt, int dirfd, const char *filename, enum TextSaveMethod type) { - if (!filename) - return NULL; - TextSave *ctx = calloc(1, sizeof *ctx); - if (!ctx) - return NULL; - ctx->txt = txt; - ctx->fd = -1; - ctx->dirfd = dirfd; - if (!(ctx->filename = strdup(filename))) - goto err; +bool text_save_begin(TextSave *ctx) { + enum TextSaveMethod type = ctx->method; errno = 0; - if ((type == TEXT_SAVE_AUTO || type == TEXT_SAVE_ATOMIC) && text_save_begin_atomic(ctx)) - return ctx; + if ((type == TEXT_SAVE_AUTO || type == TEXT_SAVE_ATOMIC) && text_save_begin_atomic(ctx)) { + ctx->method = TEXT_SAVE_ATOMIC; + return true; + } if (errno == ENOSPC) goto err; - if ((type == TEXT_SAVE_AUTO || type == TEXT_SAVE_INPLACE) && text_save_begin_inplace(ctx)) - return ctx; -err: - text_save_cancel(ctx); - return NULL; -} - -bool text_save_commit(TextSave *ctx) { - if (!ctx) + if ((type == TEXT_SAVE_AUTO || type == TEXT_SAVE_INPLACE) && text_save_begin_inplace(ctx)) { + ctx->method = TEXT_SAVE_INPLACE; return true; - bool ret; - switch (ctx->type) { - case TEXT_SAVE_ATOMIC: - ret = text_save_commit_atomic(ctx); - break; - case TEXT_SAVE_INPLACE: - ret = text_save_commit_inplace(ctx); - break; - default: - ret = false; - break; } - +err: text_save_cancel(ctx); - return ret; + return false; } void text_save_cancel(TextSave *ctx) { - if (!ctx) - return; int saved_errno = errno; if (ctx->fd != -1) close(ctx->fd); if (ctx->tmpname && ctx->tmpname[0]) unlinkat(ctx->dirfd, ctx->tmpname, 0); free(ctx->tmpname); - free(ctx->filename); - free(ctx); errno = saved_errno; } +bool text_save_commit(TextSave *ctx) { + bool result = false; + switch (ctx->method) { + case TEXT_SAVE_ATOMIC: result = text_save_commit_atomic(ctx); break; + case TEXT_SAVE_INPLACE: result = text_save_commit_inplace(ctx); break; + default: break; + } + text_save_cancel(ctx); + return result; +} + /* First try to save the file atomically using rename(2) if this does not * work overwrite the file in place. However if something goes wrong during * this overwrite the original file is permanently damaged. @@ -508,16 +474,16 @@ bool text_saveat_method(Text *txt, int dirfd, const char *filename, enum TextSav text_saved(txt, NULL); return true; } - TextSave *ctx = text_save_begin(txt, dirfd, filename, method); - if (!ctx) + TextSave ctx = text_save_default(.txt = txt, .dirfd = dirfd, .filename = filename, .method = method); + if (!text_save_begin(&ctx)) return false; Filerange range = (Filerange){ .start = 0, .end = text_size(txt) }; - ssize_t written = text_save_write_range(ctx, &range); + ssize_t written = text_save_write_range(&ctx, &range); if (written == -1 || (size_t)written != text_range_size(&range)) { - text_save_cancel(ctx); + text_save_cancel(&ctx); return false; } - return text_save_commit(ctx); + return text_save_commit(&ctx); } ssize_t text_save_write_range(TextSave *ctx, const Filerange *range) { diff --git a/text.h b/text.h index 385eb6c..08b1643 100644 --- a/text.h +++ b/text.h @@ -28,7 +28,6 @@ typedef struct { */ typedef struct Text Text; typedef struct Piece Piece; -typedef struct TextSave TextSave; /** A contiguous part of the text. */ typedef struct { @@ -350,6 +349,18 @@ enum TextSaveMethod { TEXT_SAVE_INPLACE, }; +/* used to hold context between text_save_{begin,commit} calls */ +typedef struct { + enum TextSaveMethod method; /* method used to save file */ + Text *txt; /* text to operate on */ + const char *filename; /* filename to save to */ + char *tmpname; /* temporary name used for atomic rename(2) */ + int fd; /* file descriptor to write data to using text_save_write */ + int dirfd; /* directory file descriptor, relative to which we save */ +} TextSave; + +#define text_save_default(...) (TextSave){.dirfd = AT_FDCWD, .fd = -1, __VA_ARGS__} + /** * Save the whole text to the given file name. * @@ -376,7 +387,7 @@ bool text_saveat_method(Text*, int dirfd, const char *filename, enum TextSaveMet * ``text_save_cancel`` to release the underlying resources. * @endrst */ -TextSave *text_save_begin(Text*, int dirfd, const char *filename, enum TextSaveMethod); +bool text_save_begin(TextSave*); /** * Write file range. * @return The number of bytes written or ``-1`` in case of an error. -- cgit v1.2.3