httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: apache-2.0/src/main http_log.c
Date Wed, 26 Apr 2000 12:47:25 GMT
No biggy. It's why we have diffs emailed out :-)

Cheers,
-g


On Wed, 26 Apr 2000, Bill Stoddard wrote:
> 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/
> > 
> 

-- 
Greg Stein, http://www.lyra.org/


Mime
View raw message