Propagate s/size_t/int/ in buffer_read_short_line() et al.

[Martin Neitzel]

>Submitter-Id:   net
>Originator:     Martin Neitzel
>Organization:  Gaertner Datensysteme
>Confidential:  no
>Synopsis:      Propagate  s/size_t/int/ in buffer_read_short_line() et al.
>Severity:      non-critical
>Priority:      medium
>Category:      cvs
>Class:         sw-bug
>Release:       1.12.9.1
>Environment:
        System: IRIX sco 5.3 11091810 IP12 mips
        Any ANSI C build environment.

>Description:
        Revision 1.39 of buffer.c on 2 Sep 2004 introduced
        buf_read_short_line() with size_t instead of int buffer
        lengths.  This change has been escalated to callers
        in a smaller scale in the following time.

        If taken seriously, the type change actually escalates all
        the way into high-level functions such as server.c:serve_modified()
        and reveive_file().

>How-To-Repeat:
        Compile taking warnings seriously.

>Fix:
        Well...

        I believe these patches to be correct, given one assumes
        the use of size_t length at the buf_read_short_line() level
        is correct, which I do.

        In particular, I consider the changes to resolve OK at the
        server.c level, where they kind of terminate.  The actual
        lengths are are atoi()ed from the protocol which bounds them
        at the maximum signed value which will fit into size_t.
        All's well that ends well.

        Nevertheless: do *not* apply these changes blindly but
        review the three places marked XXX.

Index: src/buffer.c
===================================================================
RCS file: /cvs/ccvs/src/buffer.c,v
retrieving revision 1.49
diff -u -r1.49 buffer.c
--- src/buffer.c        24 Sep 2004 17:55:27 -0000      1.49
+++ src/buffer.c        1 Oct 2004 14:06:47 -0000
@@ -293,7 +293,8 @@
 
        if (data->size > 0)
        {
-           int status, nbytes;
+           int status;
+           size_t nbytes;
 
            status = (*buf->output) (buf->closure, data->bufp, data->size,
                                     &nbytes);
@@ -763,8 +764,8 @@
 
     while (1)
     {
-       int get;
-       int status, nbytes;
+       size_t get, nbytes;
+       int status;
 
        if (buf->data == NULL
            || (buf->last->bufp + buf->last->size
@@ -829,7 +830,7 @@
  * Maintains LAST_INDEX & LAST_COUNT.
  */
 int
-buf_read_line (struct buffer *buf, char **line, int *lenp)
+buf_read_line (struct buffer *buf, char **line, size_t *lenp)
 {
     return buf_read_short_line (buf, line, lenp, SIZE_MAX);
 }
@@ -916,7 +917,8 @@
        predicted_len = 0;
        while (1)
        {
-           int size, status, nbytes;
+           size_t size, nbytes;
+           int status;
            char *mem;
 
            if (buf->data == NULL
@@ -985,7 +987,7 @@
  * Maintains LAST_INDEX & LAST_COUNT.
  */
 int
-buf_read_data (struct buffer *buf, int want, char **retdata, int *got)
+buf_read_data (struct buffer *buf, size_t want, char **retdata, size_t *got)
 {
     assert (buf->input != NULL);
 
@@ -1004,7 +1006,8 @@
     if (buf->data == NULL)
     {
        struct buffer_data *data;
-       int get, status, nbytes;
+       size_t get, nbytes;
+       int status;
 
        data = get_buffer_data ();
        if (data == NULL)
@@ -1343,7 +1346,7 @@
        SIZE is the amount of data in INPUT, and is also the size of
        OUTPUT.  This should return 0 on success, or an errno code.  */
     int (*inpfn) (void *fnclosure, const char *input, char *output,
-                       int size);
+                       size_t size);
     /* The output translation function.  This should translate the
        data in INPUT, storing the result in OUTPUT.  The first two
        bytes in INPUT will be the size of the data, and so will SIZE.
@@ -1351,7 +1354,7 @@
        OUTPUT.  OUTPUT is large enough to hold SIZE + PACKET_SLOP
        bytes.  This should return 0 on success, or an errno code.  */
     int (*outfn) (void *fnclosure, const char *input, char *output,
-                       int size, int *translated);
+                       size_t size, size_t *translated);
     /* A closure for the translation function.  */
     void *fnclosure;
     /* For an input buffer, we may have to buffer up data here.  */
@@ -1386,9 +1389,9 @@
 struct buffer *
 packetizing_buffer_initialize (struct buffer *buf,
                                int (*inpfn) (void *, const char *, char *,
-                                             int),
+                                             size_t),
                                int (*outfn) (void *, const char *, char *,
-                                             int, int *),
+                                             size_t, size_t *),
                                void *fnclosure,
                                void (*memory) (struct buffer *))
 {
Index: src/buffer.h
===================================================================
RCS file: /cvs/ccvs/src/buffer.h,v
retrieving revision 1.19
diff -u -r1.19 buffer.h
--- src/buffer.h        18 Sep 2004 01:16:36 -0000      1.19
+++ src/buffer.h        1 Oct 2004 14:06:47 -0000
@@ -127,8 +127,8 @@
 struct buffer *compress_buffer_initialize (struct buffer *, int, int,
                                           void (*) (struct buffer *));
 struct buffer *packetizing_buffer_initialize
-       (struct buffer *, int (*) (void *, const char *, char *, int),
-        int (*) (void *, const char *, char *, int, int *), void *,
+       (struct buffer *, int (*) (void *, const char *, char *, size_t),
+        int (*) (void *, const char *, char *, size_t, size_t *), void *,
         void (*) (struct buffer *));
 int buf_empty (struct buffer *);
 int buf_empty_p (struct buffer *);
@@ -148,10 +148,10 @@
 int buf_read_file_to_eof (FILE *, struct buffer_data **,
                          struct buffer_data **);
 int buf_input_data (struct buffer *, int *);
-int buf_read_line (struct buffer *, char **, int *);
+int buf_read_line (struct buffer *, char **, size_t *);
 int buf_read_short_line (struct buffer *buf, char **line, size_t *lenp,
                          size_t max);
-int buf_read_data (struct buffer *, int, char **, int *);
+int buf_read_data (struct buffer *, size_t, char **, size_t *);
 void buf_copy_lines (struct buffer *, struct buffer *, int);
 int buf_copy_counted (struct buffer *, struct buffer *, int *);
 int buf_chain_length (struct buffer_data *);
Index: src/client.c
===================================================================
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.385
diff -u -r1.385 client.c
--- src/client.c        29 Sep 2004 18:40:58 -0000      1.385
+++ src/client.c        1 Oct 2004 14:06:48 -0000
@@ -415,7 +415,7 @@
 {
     int status;
     char *result;
-    int len;
+    size_t len;
 
     status = buf_flush (via_to_buffer, 1);
     if (status != 0)
@@ -2984,7 +2984,8 @@
 static size_t
 try_read_from_server( char *buf, size_t len )
 {
-    int status, nread;
+    size_t nread;
+    int status;
     char *data;
 
     status = buf_read_data (global_from_server, len, &data, &nread);
Index: src/gssapi-client.c
===================================================================
RCS file: /cvs/ccvs/src/gssapi-client.c,v
retrieving revision 1.6
diff -u -r1.6 gssapi-client.c
--- src/gssapi-client.c 6 Apr 2004 20:55:50 -0000       1.6
+++ src/gssapi-client.c 1 Oct 2004 14:06:48 -0000
@@ -197,9 +197,9 @@
     gss_ctx_id_t gcontext;
 };
 
-static int cvs_gssapi_wrap_input (void *, const char *, char *, int);
-static int cvs_gssapi_wrap_output (void *, const char *, char *, int,
-                                        int *);
+static int cvs_gssapi_wrap_input (void *, const char *, char *, size_t);
+static int cvs_gssapi_wrap_output (void *, const char *, char *, size_t,
+                                        size_t *);
 
 /* Create a GSSAPI wrapping buffer.  We use a packetizing buffer with
    GSSAPI wrapping routines.  */
@@ -223,10 +223,14 @@
 
 
 /* Unwrap data using GSSAPI.  */
+/* XXX: somebody having access to an gssapi.h header:
+   please verify that the length field of an gss_buffer_desc
+   is indeed a size_t */
+   
 
 static int
 cvs_gssapi_wrap_input (void *fnclosure, const char *input, char *output,
-                       int size)
+                       size_t size)
 {
     struct cvs_gssapi_wrap_data *gd = fnclosure;
     gss_buffer_desc inbuf, outbuf;
@@ -261,7 +265,7 @@
 
 static int
 cvs_gssapi_wrap_output (void *fnclosure, const char *input, char *output,
-                        int size, int *translated)
+                        size_t size, size_t *translated)
 {
     struct cvs_gssapi_wrap_data *gd = fnclosure;
     gss_buffer_desc inbuf, outbuf;
Index: src/server.c
===================================================================
RCS file: /cvs/ccvs/src/server.c,v
retrieving revision 1.383
diff -u -r1.383 server.c
--- src/server.c        29 Sep 2004 20:47:14 -0000      1.383
+++ src/server.c        1 Oct 2004 14:06:51 -0000
@@ -1483,11 +1483,12 @@
  * enough.  Or something.
  */
 static void
-receive_partial_file (int size, int file)
+receive_partial_file (size_t size, int file)
 {
     while (size > 0)
     {
-       int status, nread;
+       size_t nread;
+       int status;
        char *data;
 
        status = buf_read_data (buf_from_net, size, &data, &nread);
@@ -1533,7 +1534,8 @@
                /* Read and discard the file data.  */
                while (size > 0)
                {
-                   int status, nread;
+                   size_t nread;
+                   int status;
                    char *data;
 
                    status = buf_read_data (buf_from_net, size, &data, &nread);
@@ -1552,7 +1554,7 @@
 
 /* Receive SIZE bytes, write to filename FILE.  */
 static void
-receive_file (int size, char *file, int gzipped)
+receive_file (size_t size, char *file, int gzipped)
 {
     int fd;
     char *arg = file;
@@ -1577,7 +1579,7 @@
           bugs).  Given that this feature is mainly for
           compatibility, that is the better tradeoff.  */
 
-       int toread = size;
+       size_t toread = size;
        char *filebuf;
        char *p;
 
@@ -1587,7 +1589,8 @@
 
        while (toread > 0)
        {
-           int status, nread;
+           size_t nread;
+           int status;
            char *data;
 
            status = buf_read_data (buf_from_net, toread, &data, &nread);
@@ -1808,7 +1811,8 @@
 static void
 serve_modified (char *arg)
 {
-    int size, status;
+    size_t size;
+    int status;
     char *size_text;
     char *mode_text;
 
@@ -1889,7 +1893,8 @@
        /* Now that we know the size, read and discard the file data.  */
        while (size > 0)
        {
-           int status, nread;
+           size_t nread;
+           int status;
            char *data;
 
            status = buf_read_data (buf_from_net, size, &data, &nread);
@@ -1911,6 +1916,8 @@
        return;
     }
 
+    /* XXX size being a size_t which is guaranteed to be unsigned,
+       the following test will always be true.  Remove the test?  */
     if (size >= 0)
     {
        receive_file (size,
@@ -2380,7 +2387,9 @@
        int status, numfds = -1;
        struct timeval *timeout_ptr;
        struct timeval timeout;
-       size_t toread;
+       /* XXX "toread" is subject to both buf_input_data(...,int*)
+          and buf_read_data(...,size_t,...).  Is this Ok? */
+       int toread;
 
        FD_ZERO (&readfds);
        FD_ZERO (&writefds);
Index: src/vers_ts.c
===================================================================
RCS file: /cvs/ccvs/src/vers_ts.c,v
retrieving revision 1.58
diff -u -r1.58 vers_ts.c
--- src/vers_ts.c       4 Sep 2004 23:25:12 -0000       1.58
+++ src/vers_ts.c       1 Oct 2004 14:06:51 -0000
@@ -355,7 +355,7 @@
 {
     struct tm *tm_p;
     char *cp;
-    int length;
+    size_t length;
 
     /* We want to use the same timestamp format as is stored in the
        st_mtime.  For unix (and NT I think) this *must* be universal
Index: src/zlib.c
===================================================================
RCS file: /cvs/ccvs/src/zlib.c,v
retrieving revision 1.22
diff -u -r1.22 zlib.c
--- src/zlib.c  5 Sep 2004 03:22:52 -0000       1.22
+++ src/zlib.c  1 Oct 2004 14:06:51 -0000
@@ -167,7 +167,8 @@
 
     while (1)
     {
-       int zstatus, sofar, status, nread;
+       int zstatus, sofar, status;
+       size_t nread;
 
        /* First try to inflate any data we already have buffered up.
           This is useful even if we don't have any buffered data,
@@ -366,7 +367,8 @@
     /* Pick up any trailing data, such as the checksum.  */
     while (1)
     {
-       int status, nread;
+       size_t nread;
+       int status;
        char buf[100];
 
        status = compress_buffer_input (cb, buf, 0, sizeof buf, &nread);