After further review, this code should work best (but please feel free
to double-check my work):
logbufflen += (len >= LOG_STR_MAX ? (len+1) : LOG_STR_MAX);
The attached patch contains this change (but is otherwise unaltered from
the original patch).
NOTE: The last message contained the exact same patch that my initial
bug report did. Sorry about resending it.
Dave
On Tue, Jul 26, 2005 at 03:14:24PM -0500, David D. Kilzer wrote:
> I believe the original patch that I submitted for this Debian bug may
> have an off-by-one error when reallocating more space for a log message
> that is longer than LOG_STR_MAX. In that case, I use the length of the
> new message but don't add one to it (meaning that there will not be room
> for the termination character). This line (which appears TWICE in the
> patch):
>
> logbufflen += (len > LOG_STR_MAX ? len : LOG_STR_MAX);
>
> should probably be something like this (note '>' to '>=' change, too):
>
> logbufflen += ((len+1) >= LOG_STR_MAX ? (len+1) : LOG_STR_MAX);
>
> It's also somewhat inefficient because it guarantees that a new buffer
> will have to be allocated the next time through the loop anyway. That's
> a different issue, though.
>
> Dave
>
>
> On Mon, Jun 20, 2005 at 01:04:34AM -0500, David D. Kilzer wrote:
>
> > Package: cvsps
> > Version: 2.0rc1-5
> > Severity: normal
> > Tags: patch
> >
> > On anoncvs.opensource.apple.com (Apple's anonymous CVS server for
> > WebKit), some very long log entries were included in CVS. I got tired
> > of cvsps-2.1 truncating them, so I made the 'logbuff' buffer be
> > dynamically allocated.
> >
> > Patch attached to implement this feature.
> >
> > -- System Information:
> > Debian Release: testing/unstable
> > APT prefers unstable
> > APT policy: (500, 'unstable')
> > Architecture: powerpc (ppc)
> > Shell: /bin/sh linked to /bin/bash
> > Kernel: Linux 2.4.20-ben7
> > Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
> >
> > Versions of packages cvsps depends on:
> > ii cvs 1:1.12.9-13 Concurrent Versions System
> > ii libc6 2.3.2.ds1-22 GNU C Library: Shared
> > libraries an
> > ii zlib1g 1:1.2.2-4 compression library - runtime
> >
> > cvsps recommends no packages.
> >
> > -- no debconf information
diff -urN cvsps-2.1-orig/cache.c cvsps-2.1/cache.c
--- cvsps-2.1-orig/cache.c 2005-05-25 22:39:40.000000000 -0500
+++ cvsps-2.1/cache.c 2005-07-26 15:21:29.716569500 -0500
@@ -108,10 +108,19 @@
int tag_flags = 0;
char branchbuff[LOG_STR_MAX] = "";
int branch_add = 0;
- char logbuff[LOG_STR_MAX] = "";
+ int logbufflen = LOG_STR_MAX + 1;
+ char * logbuff = malloc(logbufflen);
time_t cache_date = -1;
int read_version;
+ if (logbuff == NULL)
+ {
+ debug(DEBUG_SYSERROR, "could not malloc %d bytes for logbuff in
read_cache", logbufflen);
+ exit(1);
+ }
+
+ logbuff[0] = 0;
+
if (!(fp = cache_open("r")))
goto out;
@@ -299,8 +308,19 @@
else
{
/* Make sure we have enough in the buffer */
- if (strlen(logbuff)+strlen(buff)<LOG_STR_MAX)
- strcat(logbuff, buff);
+ int len = strlen(buff);
+ if (strlen(logbuff) + len >= LOG_STR_MAX)
+ {
+ logbufflen += (len >= LOG_STR_MAX ? (len+1) : LOG_STR_MAX);
+ char * newlogbuff = realloc(logbuff, logbufflen);
+ if (newlogbuff == NULL)
+ {
+ debug(DEBUG_SYSERROR, "could not realloc %d bytes for
logbuff in read_cache", logbufflen);
+ exit(1);
+ }
+ logbuff = newlogbuff;
+ }
+ strcat(logbuff, buff);
}
break;
case CACHE_NEED_PS_MEMBERS:
@@ -332,6 +352,7 @@
out_close:
fclose(fp);
out:
+ free(logbuff);
return cache_date;
}
diff -urN cvsps-2.1-orig/cvsps.c cvsps-2.1/cvsps.c
--- cvsps-2.1-orig/cvsps.c 2005-05-25 22:39:40.000000000 -0500
+++ cvsps-2.1/cvsps.c 2005-07-26 15:22:02.558230700 -0500
@@ -265,7 +265,8 @@
PatchSetMember * psm = NULL;
char datebuff[20];
char authbuff[AUTH_STR_MAX];
- char logbuff[LOG_STR_MAX + 1];
+ int logbufflen = LOG_STR_MAX + 1;
+ char * logbuff = malloc(logbufflen);
int loglen = 0;
int have_log = 0;
char cmd[BUFSIZ];
@@ -273,6 +274,12 @@
char use_rep_buff[PATH_MAX];
char * ltype;
+ if (logbuff == NULL)
+ {
+ debug(DEBUG_SYSERROR, "could not malloc %d bytes for logbuff in
load_from_cvs", logbufflen);
+ exit(1);
+ }
+
if (!no_rlog && !test_log_file && cvs_check_cap(CAP_HAVE_RLOG))
{
ltype = "rlog";
@@ -480,24 +487,22 @@
*/
if (have_log || !is_revision_metadata(buff))
{
- /* if the log buffer is full, that's it.
- *
- * Also, read lines (fgets) always have \n in them
- * which we count on. So if truncation happens,
- * be careful to put a \n on.
- *
- * Buffer has LOG_STR_MAX + 1 for room for \0 if
- * necessary
- */
- if (loglen < LOG_STR_MAX)
+ /* If the log buffer is full, try to reallocate more. */
+ if (loglen < logbufflen)
{
int len = strlen(buff);
- if (len >= LOG_STR_MAX - loglen)
+ if (len >= logbufflen - loglen)
{
- debug(DEBUG_APPMSG1, "WARNING: maximum log length
exceeded, truncating log");
- len = LOG_STR_MAX - loglen;
- buff[len - 1] = '\n';
+ debug(DEBUG_STATUS, "reallocating logbufflen to %d
bytes for file %s", logbufflen, file->filename);
+ logbufflen += (len >= LOG_STR_MAX ? (len+1) :
LOG_STR_MAX);
+ char * newlogbuff = realloc(logbuff, logbufflen);
+ if (newlogbuff == NULL)
+ {
+ debug(DEBUG_SYSERROR, "could not realloc %d
bytes for logbuff in load_from_cvs", logbufflen);
+ exit(1);
+ }
+ logbuff = newlogbuff;
}
debug(DEBUG_STATUS, "appending %s to log", buff);
