threads / patch / 53721

v2, 2 partsbugreport.c: replace strbuf_write_fd with write_in_full

Subject: [Patch v2 1/2] bugreport.c: replace strbuf_write_fd with write_in_full

## tl;dr

4 messages between Jun 19, 2020 and Jun 19, 2020. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

randall.s.becker@rogers.com· Jun 19, 2020, 20:23 UTC · lore

[Patch v2 0/2] Replace strbuf_write_fd with write_in_full

From: "Randall S. Becker" <randall.becker@nexbridge.ca>

The strbuf_write_fd method does not check whether the buffer exceeds MAX_IO_SIZE on the target platform. This fix replaces the use of that method with write_in_full, which does. Since this is the only use of strbuf_write_fd, and since the method was unsafe, it has been removed from strbuf.c and strbuf.h.

Randall S. Becker (2):
  bugreport.c: replace strbuf_write_fd with write_in_full
  strbuf: remove unreferenced strbuf_write_fd method.

 bugreport.c | 4 +++-
 strbuf.c    | 5 -----
 strbuf.h    | 1 -
 3 files changed, 3 insertions(+), 7 deletions(-)

-- 
2.21.0

randall.s.becker@rogers.com· Jun 19, 2020, 20:23 UTC · re: randall.s.becker@rogers.com · lore

From: "Randall S. Becker" <randall.becker@nexbridge.ca>

The strbuf_write_fd method did not provide checks for buffers larger than MAX_IO_SIZE. Replacing with write_in_full ensures the entire buffer will always be written to disk or report an error and die.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 bugreport.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Show changes to bugreport.c +3 −1

diff --git a/bugreport.c b/bugreport.c
index aa8a489c35..1aefa4c6bc 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -174,7 +174,9 @@ int cmd_main(int argc, const char **argv)
 		die(_("couldn't create a new file at '%s'"), report_path.buf);
 	}
 
-	strbuf_write_fd(&buffer, report);
+	if (write_in_full(report, buffer.buf, buffer.len) < 0)
+		die_errno(_("unable to write to %s"), report_path.buf);
+
 	close(report);
 
 	/*

-- 
2.21.0

randall.s.becker@rogers.com· Jun 19, 2020, 20:23 UTC · re: randall.s.becker@rogers.com · lore

[Patch v2 2/2] strbuf: remove unreferenced strbuf_write_fd method.

From: "Randall S. Becker" <randall.becker@nexbridge.ca>

strbuf_write_fd was only used in bugreport.c. Since that file now uses write_in_full, this method is no longer needed. In addition, strbuf_write_fd did not guard against exceeding MAX_IO_SIZE for the platform, nor provided error handling in the event of a failure if only partial data was written to the file descriptor. Since already write_in_full has this capability and is in general use, it should be used instead. The change impacts strbuf.c and strbuf.h.

Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
---
 strbuf.c | 5 -----
 strbuf.h | 1 -
 2 files changed, 6 deletions(-)

Show changes to 2 files +0 −6

strbuf.c, strbuf.h

diff --git a/strbuf.c b/strbuf.c
index 2f1a7d3209..e3397cc4c7 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -556,11 +556,6 @@ ssize_t strbuf_write(struct strbuf *sb, FILE *f)
 	return sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;
 }
 
-ssize_t strbuf_write_fd(struct strbuf *sb, int fd)
-{
-	return sb->len ? write(fd, sb->buf, sb->len) : 0;
-}
-
 #define STRBUF_MAXLINK (2*PATH_MAX)
 
 int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)
diff --git a/strbuf.h b/strbuf.h
index 7062eb6410..223ee2094a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -473,7 +473,6 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);
  * NUL bytes.
  */
 ssize_t strbuf_write(struct strbuf *sb, FILE *stream);
-ssize_t strbuf_write_fd(struct strbuf *sb, int fd);
 
 /**
  * Read a line from a FILE *, overwriting the existing contents of

-- 
2.21.0

Junio C Hamano· Jun 19, 2020, 20:51 UTC · re: randall.s.becker@rogers.com · lore

Re: [Patch v2 0/2] Replace strbuf_write_fd with write_in_full

randall.s.becker@rogers.com writes:

Show 7 quoted lines

> From: "Randall S. Becker" <randall.becker@nexbridge.ca>
>
> The strbuf_write_fd method does not check whether the buffer exceeds
> MAX_IO_SIZE on the target platform. This fix replaces the use of that
> method with write_in_full, which does. Since this is the only use of
> strbuf_write_fd, and since the method was unsafe, it has been removed
> from strbuf.c and strbuf.h.

Excellent.  Thanks.

Will queue.