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-06-19 21:41:25.000000000 -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 : 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-06-19 23:07:20.000000000 -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 :
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);
