httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <stodd...@raleigh.ibm.com>
Subject Re: cvs commit: apache-2.0/src/main http_log.c
Date Wed, 26 Apr 2000 12:38:49 GMT
Yea, I was in more of a hurry than I should have been.

Bill
----- Original Message ----- 
From: Greg Stein <gstein@lyra.org>
To: <new-httpd@apache.org>
Sent: Wednesday, April 26, 2000 7:03 AM
Subject: Re: cvs commit: apache-2.0/src/main http_log.c


> On 26 Apr 2000 stoddard@locus.apache.org wrote:
> >...
> >   -const char *ap_strerror(ap_status_t statcode, ap_pool_t *p)
> >   +char *ap_strerror(ap_status_t statcode, char *buf, size_t bufsize)
> >    {
> >        if (statcode < APR_OS_START_ERROR) {
> >            return strerror(statcode);
> >   @@ -151,7 +151,7 @@
> >            return "APR does not understand this error code";
> >        }
> >        else {
> >   - return apr_os_strerror(statcode - APR_OS_START_SYSERR, p);
> >   + return apr_os_strerror(statcode - APR_OS_START_SYSERR, buf, bufsize);
> >        }
> >    }
> 
> Shouldn't we be copying the strings INTO the provided buffer?
> 
> In three of the cases of ap_strerror(), we return a constant string and
> never place anything into the buffer. Two problems:
> 
> 1) it is a constant. if somebody tries to jam something into the "char *"
>    that we return, it could segfault.
> 2) the caller may expect the error to go into the buffer, and ignore the
>    return value.
> 
> The return value is kind of a convenience, is all. If it will be different
> from "return buf;", then we should add a const to it.
> (I also wouldn't be opposed to a void function if we always copied to buf)
> 
> >...
> >   @@ -162,7 +162,7 @@
> >    #include <os2.h>
> >    #include <ctype.h>
> >    
> >   -static const char *apr_os_strerror(int err, ap_pool_t *p)
> >   +static char *apr_os_strerror(int err, char* buf, size_t bufsize)
> >    {
> >      char result[200];
> >      unsigned char message[HUGE_STRING_LEN];
> >   @@ -195,6 +195,11 @@
> >          sprintf(result, "OS/2 error %d", err);
> >      }
> >      
> >   -  return ap_pstrdup(p, result);
> >   +  if (len > bufsize)
> >   +      len = bufsize;
> >   +
> >   +  memcpy(buf, result, len);
> >   +
> >   +  return buf;
> 
> That string may not be null-terminated. I'd recommend adding a
> "buf[len-1] = '\0';" after the memcpy().
> 
> Cheers,
> -g
> 
> -- 
> Greg Stein, http://www.lyra.org/
> 


Mime
View raw message