[PATCH] remove unnecessary test and dead diagnostic

Subsystems: the rest

DORMANTno replies

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH] remove unnecessary test and dead diagnostic

From: Jim Meyering <hidden>
Date: 2016-06-15 22:51:19

* sha1_file.c (index_stream): Don't check for size_t < 0.
read_in_full does not return an indication of failure.

Signed-off-by: Jim Meyering <redacted>
---
 sha1_file.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 5fc877f..ea4549c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2736,8 +2736,6 @@ static int index_stream(unsigned char *sha1, int fd, size_t size,
 		size_t actual;

 		actual = read_in_full(fd, buf, sz);
-		if (actual < 0)
-			die_errno("index-stream: reading input");
 		if (write_in_full(fast_import.in, buf, actual) != actual)
 			die_errno("index-stream: feeding fast-import");
 		size -= actual;
--
1.7.5.2.660.g9f46c

Re: [PATCH] remove unnecessary test and dead diagnostic

From: Jeff King <hidden>
Date: 2016-06-15 22:51:19

On Thu, May 26, 2011 at 03:59:14PM +0200, Jim Meyering wrote:
* sha1_file.c (index_stream): Don't check for size_t < 0.
read_in_full does not return an indication of failure.
Are you sure about that?

  $ sed -n '/read_in_full/,/^}/p' wrapper.c
  ssize_t read_in_full(int fd, void *buf, size_t count)
  {
          char *p = buf;
          ssize_t total = 0;

          while (count > 0) {
                  ssize_t loaded = xread(fd, p, count);
                  if (loaded <= 0)
                          return total ? total : loaded;
                  count -= loaded;
                  p += loaded;
                  total += loaded;
          }

          return total;
  }

It looks like if we get -1 on the _first_ read, we will then return -1.
Subsequent errors are then ignored, and we return the (possibly
truncated) result.

Which, to be honest, seems kind of insane to me. I'd think:

  while (count > 0) {
          ssize_t loaded = xread(fd, p, count);
          if (loaded < 0)
                  return loaded;
          if (loaded == 0)
                  return total;
          ...
  }

would be much more sensible semantics.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help