* BUG in fixup_pack_header_footer(...) / pack-write.c checksum error never raised @ 2007-05-15 11:47 Paolo Teti 2007-05-15 12:39 ` [PATCH] Fix signedness on return value from xread() Johan Herland 0 siblings, 1 reply; 7+ messages in thread From: Paolo Teti @ 2007-05-15 11:47 UTC (permalink / raw) To: git In fixup_pack_header_footer(...) file pack-write.c We have to change the size_t value returned by xread() in ssize_t, otherwise the next check on negative values has no sense. size_t is an unsigned type! I can't write now a patch because I'm on a customer site and I can't install GIT... I'm just looking at the souce code using gitweb during a coffebreak. Someone can fix it?.. Pao ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Fix signedness on return value from xread() 2007-05-15 11:47 BUG in fixup_pack_header_footer(...) / pack-write.c checksum error never raised Paolo Teti @ 2007-05-15 12:39 ` Johan Herland 2007-05-15 12:48 ` Paolo Teti ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Johan Herland @ 2007-05-15 12:39 UTC (permalink / raw) To: git; +Cc: Paolo Teti The return value from xread() is ssize_t. Paolo Teti <paolo.teti@gmail.com> pointed out that in this case, the signed return value was assigned to an unsigned type (size_t). This patch fixes that. Signed-off-by: Johan Herland <johan@herland.net> --- Using this as an opportunity to get used to sending patches... :) Is this what you were looking for, Paolo? pack-write.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/pack-write.c b/pack-write.c index de72f44..ae2e481 100644 --- a/pack-write.c +++ b/pack-write.c @@ -25,7 +25,7 @@ void fixup_pack_header_footer(int pack_fd, buf = xmalloc(buf_sz); for (;;) { - size_t n = xread(pack_fd, buf, buf_sz); + ssize_t n = xread(pack_fd, buf, buf_sz); if (!n) break; if (n < 0) -- 1.5.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix signedness on return value from xread() 2007-05-15 12:39 ` [PATCH] Fix signedness on return value from xread() Johan Herland @ 2007-05-15 12:48 ` Paolo Teti 2007-05-15 12:49 ` [PATCH] Ensure return value from xread() is always stored into an ssize_t Johan Herland 2007-05-15 23:24 ` [PATCH] Fix signedness on return value from xread() Shawn O. Pearce 2 siblings, 0 replies; 7+ messages in thread From: Paolo Teti @ 2007-05-15 12:48 UTC (permalink / raw) To: Johan Herland; +Cc: git 2007/5/15, Johan Herland <johan@herland.net>: > Using this as an opportunity to get used to sending patches... :) > > Is this what you were looking for, Paolo? > Yes Johan your patch fixes the bug. Thanks ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] Ensure return value from xread() is always stored into an ssize_t 2007-05-15 12:39 ` [PATCH] Fix signedness on return value from xread() Johan Herland 2007-05-15 12:48 ` Paolo Teti @ 2007-05-15 12:49 ` Johan Herland 2007-05-15 16:12 ` Dana How 2007-05-15 23:24 ` [PATCH] Fix signedness on return value from xread() Shawn O. Pearce 2 siblings, 1 reply; 7+ messages in thread From: Johan Herland @ 2007-05-15 12:49 UTC (permalink / raw) To: git; +Cc: Paolo Teti This patch fixes all calls to xread() where the return value is not stored into an ssize_t. The patch should not have any effect whatsoever, other than putting better/more appropriate type names on variables. Signed-off-by: Johan Herland <johan@herland.net> --- Feel free to ignore this patch as it may be argued that it needlessly touches a lot of code, without having much (if any) positive effect at all. builtin-apply.c | 4 ++-- builtin-bundle.c | 2 +- builtin-fetch--tool.c | 4 ++-- builtin-unpack-objects.c | 2 +- combine-diff.c | 2 +- copy.c | 3 +-- diff.c | 2 +- imap-send.c | 2 +- index-pack.c | 2 +- pkt-line.c | 4 ++-- sha1_file.c | 2 +- ssh-upload.c | 2 +- 12 files changed, 15 insertions(+), 16 deletions(-) diff --git a/builtin-apply.c b/builtin-apply.c index 8b8705a..0399743 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -185,7 +185,7 @@ static void *read_patch_file(int fd, unsigned long *sizep) void *buffer = xmalloc(alloc); for (;;) { - int nr = alloc - size; + ssize_t nr = alloc - size; if (nr < 1024) { alloc += CHUNKSIZE; buffer = xrealloc(buffer, alloc); @@ -1468,7 +1468,7 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign return error("unable to open %s", path); got = 0; for (;;) { - int ret = xread(fd, buf + got, size - got); + ssize_t ret = xread(fd, buf + got, size - got); if (ret <= 0) break; got += ret; diff --git a/builtin-bundle.c b/builtin-bundle.c index d1635a0..306ad29 100644 --- a/builtin-bundle.c +++ b/builtin-bundle.c @@ -48,7 +48,7 @@ static int read_string(int fd, char *buffer, int size) { int i; for (i = 0; i < size - 1; i++) { - int count = xread(fd, buffer + i, 1); + ssize_t count = xread(fd, buffer + i, 1); if (count < 0) return error("Read error: %s", strerror(errno)); if (count == 0) { diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c index 2065466..12adb38 100644 --- a/builtin-fetch--tool.c +++ b/builtin-fetch--tool.c @@ -6,11 +6,11 @@ static char *get_stdin(void) { - int offset = 0; + size_t offset = 0; char *data = xmalloc(CHUNK_SIZE); while (1) { - int cnt = xread(0, data + offset, CHUNK_SIZE); + ssize_t cnt = xread(0, data + offset, CHUNK_SIZE); if (cnt < 0) die("error reading standard input: %s", strerror(errno)); diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c index 2bbda67..a6ff62f 100644 --- a/builtin-unpack-objects.c +++ b/builtin-unpack-objects.c @@ -34,7 +34,7 @@ static void *fill(int min) offset = 0; } do { - int ret = xread(0, buffer + len, sizeof(buffer) - len); + ssize_t ret = xread(0, buffer + len, sizeof(buffer) - len); if (ret <= 0) { if (!ret) die("early EOF"); diff --git a/combine-diff.c b/combine-diff.c index cff9c5d..ea3ca5f 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -714,7 +714,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, result_size = len; result = xmalloc(len + 1); while (sz < len) { - int done = xread(fd, result+sz, len-sz); + ssize_t done = xread(fd, result+sz, len-sz); if (done == 0) break; if (done < 0) diff --git a/copy.c b/copy.c index 08a3d38..d340bb2 100644 --- a/copy.c +++ b/copy.c @@ -3,10 +3,9 @@ int copy_fd(int ifd, int ofd) { while (1) { - int len; char buffer[8192]; char *buf = buffer; - len = xread(ifd, buffer, sizeof(buffer)); + ssize_t len = xread(ifd, buffer, sizeof(buffer)); if (!len) break; if (len < 0) { diff --git a/diff.c b/diff.c index 8354e71..33297aa 100644 --- a/diff.c +++ b/diff.c @@ -1411,7 +1411,7 @@ static int populate_from_stdin(struct diff_filespec *s) #define INCREMENT 1024 char *buf; unsigned long size; - int got; + ssize_t got; size = 0; buf = NULL; diff --git a/imap-send.c b/imap-send.c index 84df2fa..4283a4a 100644 --- a/imap-send.c +++ b/imap-send.c @@ -224,7 +224,7 @@ socket_perror( const char *func, Socket_t *sock, int ret ) static int socket_read( Socket_t *sock, char *buf, int len ) { - int n = xread( sock->fd, buf, len ); + ssize_t n = xread( sock->fd, buf, len ); if (n <= 0) { socket_perror( "read", sock, n ); close( sock->fd ); diff --git a/index-pack.c b/index-pack.c index b9da19f..58c4a9c 100644 --- a/index-pack.c +++ b/index-pack.c @@ -82,7 +82,7 @@ static void *fill(int min) die("cannot fill %d bytes", min); flush(); do { - int ret = xread(input_fd, input_buffer + input_len, + ssize_t ret = xread(input_fd, input_buffer + input_len, sizeof(input_buffer) - input_len); if (ret <= 0) { if (!ret) diff --git a/pkt-line.c b/pkt-line.c index b4cb7e2..b605268 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -65,10 +65,10 @@ void packet_write(int fd, const char *fmt, ...) static void safe_read(int fd, void *buffer, unsigned size) { - int n = 0; + size_t n = 0; while (n < size) { - int ret = xread(fd, (char *) buffer + n, size - n); + ssize_t ret = xread(fd, (char *) buffer + n, size - n); if (ret < 0) die("read error (%s)", strerror(errno)); if (!ret) diff --git a/sha1_file.c b/sha1_file.c index 32244d7..be991ed 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2276,7 +2276,7 @@ int read_pipe(int fd, char** return_buf, unsigned long* return_size) { char* buf = *return_buf; unsigned long size = *return_size; - int iret; + ssize_t iret; unsigned long off = 0; do { diff --git a/ssh-upload.c b/ssh-upload.c index 2f04572..498d41e 100644 --- a/ssh-upload.c +++ b/ssh-upload.c @@ -86,7 +86,7 @@ static int serve_ref(int fd_in, int fd_out) static void service(int fd_in, int fd_out) { char type; - int retval; + ssize_t retval; do { retval = xread(fd_in, &type, 1); if (retval < 1) { -- 1.5.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Ensure return value from xread() is always stored into an ssize_t 2007-05-15 12:49 ` [PATCH] Ensure return value from xread() is always stored into an ssize_t Johan Herland @ 2007-05-15 16:12 ` Dana How 2007-05-15 16:21 ` Johan Herland 0 siblings, 1 reply; 7+ messages in thread From: Dana How @ 2007-05-15 16:12 UTC (permalink / raw) To: Johan Herland; +Cc: git, Paolo Teti, danahow Twice you change an int to a size_t (one s). [builtin-fetch-tool.c and pkt-line.c .] Did you want to do that? On 5/15/07, Johan Herland <johan@herland.net> wrote: > This patch fixes all calls to xread() where the return value is not > stored into an ssize_t. The patch should not have any effect whatsoever, > other than putting better/more appropriate type names on variables. > > Signed-off-by: Johan Herland <johan@herland.net> > --- > > Feel free to ignore this patch as it may be argued that it needlessly > touches a lot of code, without having much (if any) positive effect > at all. > > builtin-apply.c | 4 ++-- > builtin-bundle.c | 2 +- > builtin-fetch--tool.c | 4 ++-- > builtin-unpack-objects.c | 2 +- > combine-diff.c | 2 +- > copy.c | 3 +-- > diff.c | 2 +- > imap-send.c | 2 +- > index-pack.c | 2 +- > pkt-line.c | 4 ++-- > sha1_file.c | 2 +- > ssh-upload.c | 2 +- > 12 files changed, 15 insertions(+), 16 deletions(-) > > diff --git a/builtin-apply.c b/builtin-apply.c > index 8b8705a..0399743 100644 > --- a/builtin-apply.c > +++ b/builtin-apply.c > @@ -185,7 +185,7 @@ static void *read_patch_file(int fd, unsigned long *sizep) > void *buffer = xmalloc(alloc); > > for (;;) { > - int nr = alloc - size; > + ssize_t nr = alloc - size; > if (nr < 1024) { > alloc += CHUNKSIZE; > buffer = xrealloc(buffer, alloc); > @@ -1468,7 +1468,7 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign > return error("unable to open %s", path); > got = 0; > for (;;) { > - int ret = xread(fd, buf + got, size - got); > + ssize_t ret = xread(fd, buf + got, size - got); > if (ret <= 0) > break; > got += ret; > diff --git a/builtin-bundle.c b/builtin-bundle.c > index d1635a0..306ad29 100644 > --- a/builtin-bundle.c > +++ b/builtin-bundle.c > @@ -48,7 +48,7 @@ static int read_string(int fd, char *buffer, int size) > { > int i; > for (i = 0; i < size - 1; i++) { > - int count = xread(fd, buffer + i, 1); > + ssize_t count = xread(fd, buffer + i, 1); > if (count < 0) > return error("Read error: %s", strerror(errno)); > if (count == 0) { > diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c > index 2065466..12adb38 100644 > --- a/builtin-fetch--tool.c > +++ b/builtin-fetch--tool.c > @@ -6,11 +6,11 @@ > > static char *get_stdin(void) > { > - int offset = 0; > + size_t offset = 0; > char *data = xmalloc(CHUNK_SIZE); > > while (1) { > - int cnt = xread(0, data + offset, CHUNK_SIZE); > + ssize_t cnt = xread(0, data + offset, CHUNK_SIZE); > if (cnt < 0) > die("error reading standard input: %s", > strerror(errno)); > diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c > index 2bbda67..a6ff62f 100644 > --- a/builtin-unpack-objects.c > +++ b/builtin-unpack-objects.c > @@ -34,7 +34,7 @@ static void *fill(int min) > offset = 0; > } > do { > - int ret = xread(0, buffer + len, sizeof(buffer) - len); > + ssize_t ret = xread(0, buffer + len, sizeof(buffer) - len); > if (ret <= 0) { > if (!ret) > die("early EOF"); > diff --git a/combine-diff.c b/combine-diff.c > index cff9c5d..ea3ca5f 100644 > --- a/combine-diff.c > +++ b/combine-diff.c > @@ -714,7 +714,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, > result_size = len; > result = xmalloc(len + 1); > while (sz < len) { > - int done = xread(fd, result+sz, len-sz); > + ssize_t done = xread(fd, result+sz, len-sz); > if (done == 0) > break; > if (done < 0) > diff --git a/copy.c b/copy.c > index 08a3d38..d340bb2 100644 > --- a/copy.c > +++ b/copy.c > @@ -3,10 +3,9 @@ > int copy_fd(int ifd, int ofd) > { > while (1) { > - int len; > char buffer[8192]; > char *buf = buffer; > - len = xread(ifd, buffer, sizeof(buffer)); > + ssize_t len = xread(ifd, buffer, sizeof(buffer)); > if (!len) > break; > if (len < 0) { > diff --git a/diff.c b/diff.c > index 8354e71..33297aa 100644 > --- a/diff.c > +++ b/diff.c > @@ -1411,7 +1411,7 @@ static int populate_from_stdin(struct diff_filespec *s) > #define INCREMENT 1024 > char *buf; > unsigned long size; > - int got; > + ssize_t got; > > size = 0; > buf = NULL; > diff --git a/imap-send.c b/imap-send.c > index 84df2fa..4283a4a 100644 > --- a/imap-send.c > +++ b/imap-send.c > @@ -224,7 +224,7 @@ socket_perror( const char *func, Socket_t *sock, int ret ) > static int > socket_read( Socket_t *sock, char *buf, int len ) > { > - int n = xread( sock->fd, buf, len ); > + ssize_t n = xread( sock->fd, buf, len ); > if (n <= 0) { > socket_perror( "read", sock, n ); > close( sock->fd ); > diff --git a/index-pack.c b/index-pack.c > index b9da19f..58c4a9c 100644 > --- a/index-pack.c > +++ b/index-pack.c > @@ -82,7 +82,7 @@ static void *fill(int min) > die("cannot fill %d bytes", min); > flush(); > do { > - int ret = xread(input_fd, input_buffer + input_len, > + ssize_t ret = xread(input_fd, input_buffer + input_len, > sizeof(input_buffer) - input_len); > if (ret <= 0) { > if (!ret) > diff --git a/pkt-line.c b/pkt-line.c > index b4cb7e2..b605268 100644 > --- a/pkt-line.c > +++ b/pkt-line.c > @@ -65,10 +65,10 @@ void packet_write(int fd, const char *fmt, ...) > > static void safe_read(int fd, void *buffer, unsigned size) > { > - int n = 0; > + size_t n = 0; > > while (n < size) { > - int ret = xread(fd, (char *) buffer + n, size - n); > + ssize_t ret = xread(fd, (char *) buffer + n, size - n); > if (ret < 0) > die("read error (%s)", strerror(errno)); > if (!ret) > diff --git a/sha1_file.c b/sha1_file.c > index 32244d7..be991ed 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -2276,7 +2276,7 @@ int read_pipe(int fd, char** return_buf, unsigned long* return_size) > { > char* buf = *return_buf; > unsigned long size = *return_size; > - int iret; > + ssize_t iret; > unsigned long off = 0; > > do { > diff --git a/ssh-upload.c b/ssh-upload.c > index 2f04572..498d41e 100644 > --- a/ssh-upload.c > +++ b/ssh-upload.c > @@ -86,7 +86,7 @@ static int serve_ref(int fd_in, int fd_out) > > static void service(int fd_in, int fd_out) { > char type; > - int retval; > + ssize_t retval; > do { > retval = xread(fd_in, &type, 1); > if (retval < 1) { > -- > 1.5.1.4 > - > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Dana L. How danahow@gmail.com +1 650 804 5991 cell ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Ensure return value from xread() is always stored into an ssize_t 2007-05-15 16:12 ` Dana How @ 2007-05-15 16:21 ` Johan Herland 0 siblings, 0 replies; 7+ messages in thread From: Johan Herland @ 2007-05-15 16:21 UTC (permalink / raw) To: Dana How; +Cc: git, Paolo Teti On Tuesday 15 May 2007, Dana How wrote: > Twice you change an int to a size_t (one s). > [builtin-fetch-tool.c and pkt-line.c .] > Did you want to do that? Yes. AFAICS it was appropriate in the given context. I.e. in both cases the int was used as an positive offset/size, and although its value was altered by the ssize_t returned from xread(), the ssize_t was always verified to be >= 0 before being added to the int. Have fun! ...Johan > On 5/15/07, Johan Herland <johan@herland.net> wrote: > > This patch fixes all calls to xread() where the return value is not > > stored into an ssize_t. The patch should not have any effect whatsoever, > > other than putting better/more appropriate type names on variables. > > > > Signed-off-by: Johan Herland <johan@herland.net> > > --- > > > > Feel free to ignore this patch as it may be argued that it needlessly > > touches a lot of code, without having much (if any) positive effect > > at all. > > > > builtin-apply.c | 4 ++-- > > builtin-bundle.c | 2 +- > > builtin-fetch--tool.c | 4 ++-- > > builtin-unpack-objects.c | 2 +- > > combine-diff.c | 2 +- > > copy.c | 3 +-- > > diff.c | 2 +- > > imap-send.c | 2 +- > > index-pack.c | 2 +- > > pkt-line.c | 4 ++-- > > sha1_file.c | 2 +- > > ssh-upload.c | 2 +- > > 12 files changed, 15 insertions(+), 16 deletions(-) > > > > diff --git a/builtin-apply.c b/builtin-apply.c > > index 8b8705a..0399743 100644 > > --- a/builtin-apply.c > > +++ b/builtin-apply.c > > @@ -185,7 +185,7 @@ static void *read_patch_file(int fd, unsigned long *sizep) > > void *buffer = xmalloc(alloc); > > > > for (;;) { > > - int nr = alloc - size; > > + ssize_t nr = alloc - size; > > if (nr < 1024) { > > alloc += CHUNKSIZE; > > buffer = xrealloc(buffer, alloc); > > @@ -1468,7 +1468,7 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign > > return error("unable to open %s", path); > > got = 0; > > for (;;) { > > - int ret = xread(fd, buf + got, size - got); > > + ssize_t ret = xread(fd, buf + got, size - got); > > if (ret <= 0) > > break; > > got += ret; > > diff --git a/builtin-bundle.c b/builtin-bundle.c > > index d1635a0..306ad29 100644 > > --- a/builtin-bundle.c > > +++ b/builtin-bundle.c > > @@ -48,7 +48,7 @@ static int read_string(int fd, char *buffer, int size) > > { > > int i; > > for (i = 0; i < size - 1; i++) { > > - int count = xread(fd, buffer + i, 1); > > + ssize_t count = xread(fd, buffer + i, 1); > > if (count < 0) > > return error("Read error: %s", strerror(errno)); > > if (count == 0) { > > diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c > > index 2065466..12adb38 100644 > > --- a/builtin-fetch--tool.c > > +++ b/builtin-fetch--tool.c > > @@ -6,11 +6,11 @@ > > > > static char *get_stdin(void) > > { > > - int offset = 0; > > + size_t offset = 0; > > char *data = xmalloc(CHUNK_SIZE); > > > > while (1) { > > - int cnt = xread(0, data + offset, CHUNK_SIZE); > > + ssize_t cnt = xread(0, data + offset, CHUNK_SIZE); > > if (cnt < 0) > > die("error reading standard input: %s", > > strerror(errno)); > > diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c > > index 2bbda67..a6ff62f 100644 > > --- a/builtin-unpack-objects.c > > +++ b/builtin-unpack-objects.c > > @@ -34,7 +34,7 @@ static void *fill(int min) > > offset = 0; > > } > > do { > > - int ret = xread(0, buffer + len, sizeof(buffer) - len); > > + ssize_t ret = xread(0, buffer + len, sizeof(buffer) - len); > > if (ret <= 0) { > > if (!ret) > > die("early EOF"); > > diff --git a/combine-diff.c b/combine-diff.c > > index cff9c5d..ea3ca5f 100644 > > --- a/combine-diff.c > > +++ b/combine-diff.c > > @@ -714,7 +714,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, > > result_size = len; > > result = xmalloc(len + 1); > > while (sz < len) { > > - int done = xread(fd, result+sz, len-sz); > > + ssize_t done = xread(fd, result+sz, len-sz); > > if (done == 0) > > break; > > if (done < 0) > > diff --git a/copy.c b/copy.c > > index 08a3d38..d340bb2 100644 > > --- a/copy.c > > +++ b/copy.c > > @@ -3,10 +3,9 @@ > > int copy_fd(int ifd, int ofd) > > { > > while (1) { > > - int len; > > char buffer[8192]; > > char *buf = buffer; > > - len = xread(ifd, buffer, sizeof(buffer)); > > + ssize_t len = xread(ifd, buffer, sizeof(buffer)); > > if (!len) > > break; > > if (len < 0) { > > diff --git a/diff.c b/diff.c > > index 8354e71..33297aa 100644 > > --- a/diff.c > > +++ b/diff.c > > @@ -1411,7 +1411,7 @@ static int populate_from_stdin(struct diff_filespec *s) > > #define INCREMENT 1024 > > char *buf; > > unsigned long size; > > - int got; > > + ssize_t got; > > > > size = 0; > > buf = NULL; > > diff --git a/imap-send.c b/imap-send.c > > index 84df2fa..4283a4a 100644 > > --- a/imap-send.c > > +++ b/imap-send.c > > @@ -224,7 +224,7 @@ socket_perror( const char *func, Socket_t *sock, int ret ) > > static int > > socket_read( Socket_t *sock, char *buf, int len ) > > { > > - int n = xread( sock->fd, buf, len ); > > + ssize_t n = xread( sock->fd, buf, len ); > > if (n <= 0) { > > socket_perror( "read", sock, n ); > > close( sock->fd ); > > diff --git a/index-pack.c b/index-pack.c > > index b9da19f..58c4a9c 100644 > > --- a/index-pack.c > > +++ b/index-pack.c > > @@ -82,7 +82,7 @@ static void *fill(int min) > > die("cannot fill %d bytes", min); > > flush(); > > do { > > - int ret = xread(input_fd, input_buffer + input_len, > > + ssize_t ret = xread(input_fd, input_buffer + input_len, > > sizeof(input_buffer) - input_len); > > if (ret <= 0) { > > if (!ret) > > diff --git a/pkt-line.c b/pkt-line.c > > index b4cb7e2..b605268 100644 > > --- a/pkt-line.c > > +++ b/pkt-line.c > > @@ -65,10 +65,10 @@ void packet_write(int fd, const char *fmt, ...) > > > > static void safe_read(int fd, void *buffer, unsigned size) > > { > > - int n = 0; > > + size_t n = 0; > > > > while (n < size) { > > - int ret = xread(fd, (char *) buffer + n, size - n); > > + ssize_t ret = xread(fd, (char *) buffer + n, size - n); > > if (ret < 0) > > die("read error (%s)", strerror(errno)); > > if (!ret) > > diff --git a/sha1_file.c b/sha1_file.c > > index 32244d7..be991ed 100644 > > --- a/sha1_file.c > > +++ b/sha1_file.c > > @@ -2276,7 +2276,7 @@ int read_pipe(int fd, char** return_buf, unsigned long* return_size) > > { > > char* buf = *return_buf; > > unsigned long size = *return_size; > > - int iret; > > + ssize_t iret; > > unsigned long off = 0; > > > > do { > > diff --git a/ssh-upload.c b/ssh-upload.c > > index 2f04572..498d41e 100644 > > --- a/ssh-upload.c > > +++ b/ssh-upload.c > > @@ -86,7 +86,7 @@ static int serve_ref(int fd_in, int fd_out) > > > > static void service(int fd_in, int fd_out) { > > char type; > > - int retval; > > + ssize_t retval; > > do { > > retval = xread(fd_in, &type, 1); > > if (retval < 1) { > > -- > > 1.5.1.4 > > - > > To unsubscribe from this list: send the line "unsubscribe git" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Johan Herland, <johan@herland.net> www.herland.net ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix signedness on return value from xread() 2007-05-15 12:39 ` [PATCH] Fix signedness on return value from xread() Johan Herland 2007-05-15 12:48 ` Paolo Teti 2007-05-15 12:49 ` [PATCH] Ensure return value from xread() is always stored into an ssize_t Johan Herland @ 2007-05-15 23:24 ` Shawn O. Pearce 2 siblings, 0 replies; 7+ messages in thread From: Shawn O. Pearce @ 2007-05-15 23:24 UTC (permalink / raw) To: Johan Herland; +Cc: git, Paolo Teti Johan Herland <johan@herland.net> wrote: > The return value from xread() is ssize_t. > Paolo Teti <paolo.teti@gmail.com> pointed out that in this case, the > signed return value was assigned to an unsigned type (size_t). This patch > fixes that. Thanks for fixing this. Its rather embarrassing as: commit 00be8dcc1aca3c1c1a94b39f0563d30d1fa89290 Author: Sami Farin <safari-kernel@safari.iki.fi> Date: Tue Apr 24 22:56:02 2007 +0300 fast-import: size_t vs ssize_t size_t is unsigned, so (n < 0) is never true. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> was applied by me to fix this very bug and I also applied the patch that created the buggy copy in pack-write.c. -- Shawn. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-05-15 23:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-05-15 11:47 BUG in fixup_pack_header_footer(...) / pack-write.c checksum error never raised Paolo Teti 2007-05-15 12:39 ` [PATCH] Fix signedness on return value from xread() Johan Herland 2007-05-15 12:48 ` Paolo Teti 2007-05-15 12:49 ` [PATCH] Ensure return value from xread() is always stored into an ssize_t Johan Herland 2007-05-15 16:12 ` Dana How 2007-05-15 16:21 ` Johan Herland 2007-05-15 23:24 ` [PATCH] Fix signedness on return value from xread() Shawn O. Pearce
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).