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

[Derek Robert Price]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Martin,

Alexander Taler ended up sending a more complete size_t update, but I
did eliminate the size >= 0 check you pointed out.  Thanks for the
report!  The change will be released in 1.12.10.

Cheers,

Derek

Martin Neitzel wrote:

>>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);
>
>
>_______________________________________________
>Bug-cvs mailing list
>Bug-cvs@gnu.org
>http://lists.gnu.org/mailman/listinfo/bug-cvs



- --
                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFBdCbqLD1OTBfyMaQRAkyOAJ9rtuniPuDNFRkXQ7cHizfoGALj4wCeM/Vo
gyc3dthyBRc55wJ/qqszsGw=
=pC70
-----END PGP SIGNATURE-----