[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
cvs 1.12 expand_path variable expansion bug
From: |
Dan Riley |
Subject: |
cvs 1.12 expand_path variable expansion bug |
Date: |
Fri, 23 Jul 2004 16:43:50 -0400 |
On alphaev67-dec-osf4.0f, cvs 1.12.7 and 1.12.9, configured with
./configure '--enable-encryption' '--with-external-zlib' '--with-gssapi' 'CC=cc
-std' 'CFLAGS=-fast' 'CPPFLAGS=-I/usr/local/include' 'LDFLAGS=-L/usr/local/lib'
with this at line 32 of CVSROOT/loginfo:
ALL $CVSROOT/CVSROOT/log.pl -m dsr -m dab66 -m crn -f
$CVSROOT/CVSROOT/commitlog %s
(also with s/%s/%{s}/), every cvs commit prints a message like
cvs commit: loginfo:32: no such internal variable $Cal/bin/túÿÿÿÿÿÿÿ
or
cvs commit: loginfo:32: no such internal variable $C
This did not happen with this line:
ALL $CVSROOT/CVSROOT/log.pl -m dsr -m dab66 -f $CVSROOT/CVSROOT/commitlog %s
(shorter by 7 characters) and does not happen with cvs 1.11.17.
with a little debugging:
(ladebug) where
#0 0x12003207c in
expand_variable(name=0x14004203f="C.lns.cor\372\377\377\377\377\377\377\377",
file=0x140025bf8="loginfo", line=32) "expand_path.c":333
>1 0x120031adc in expand_path(name=0x140037b64="$CVSROOT/CVSROOT/log.pl -m dsr
>-m dab66 -m crn -f $CVSROOT/CVSROOT/commitlog %s", file=0x140025bf8="loginfo",
>line=32, formatsafe=1) "expand_path.c":177
[...]
and in expand_path, p is no longer in mybuf:
(ladebug) p p
0x14004203f="C.lns.cor\372\377\377\377\377\377\377\377"
(ladebug) p mybuf
0x1400420a0="/home/cmpgrp/cvsroot/CVSROOT/log.pl -m dsr -m dab66 -m crn -f
$CVSROOT"
because in this chunk of code:
char *p = d;
char *e;
int flag = (*s == '{');
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
d = mybuf + doff;
for (; (*d++ = *s); s++)
{
if (flag
? *s =='}'
: isalnum ((unsigned char) *s) == 0 && *s != '_')
break;
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
d = mybuf + doff;
}
*--d = '\0';
e = expand_variable (&p[flag], file, line);
p depends on mybuf not getting relocated. Below is a proposed patch,
which changes p to an offset inside mybuf, similar to doff. I have
not audited the routine for any other mistakes of this sort, but on
casual inspection it looks like the next block has the same problem
wrt the variable e:
if (e)
{
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
d = mybuf + doff;
for (d = &mybuf[poff-1]; (*d++ = *e++);)
so I'd say someone needs to go over the whole routine carefully. My
officemate suggests rewriting it in a language with a proper string
type--he recommends C++ :-). A less radical possibility would be to
eliminate the pointer arithmetic and only use array style indexing
with mybuf[].
This patch also contains an unrelated fix, changing
*d++ = d[-1];
to
*d = d[-1];
++d;
The former is undefined behavior--the value of d on the rhs is
undefined because there's no sequence point between the auto-increment
and the rhs. "*d++ = '%';" would also work.
--- src/expand_path.c~ 2004-03-22 10:37:34.000000000 -0500
+++ src/expand_path.c 2004-07-23 16:07:00.000000000 -0400
@@ -156,7 +156,7 @@
}
else if (*s++ == '$')
{
- char *p = d;
+ char poff = d - mybuf;
char *e;
int flag = (*s == '{');
@@ -174,14 +174,14 @@
d = mybuf + doff;
}
*--d = '\0';
- e = expand_variable (&p[flag], file, line);
+ e = expand_variable (&mybuf[poff+flag], file, line);
if (e)
{
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
d = mybuf + doff;
- for (d = &p[-1]; (*d++ = *e++);)
+ for (d = &mybuf[poff-1]; (*d++ = *e++);)
{
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
@@ -209,7 +209,8 @@
doff = d - mybuf;
expand_string (&mybuf, &mybuf_size, doff + 1);
d = mybuf + doff;
- *d++ = d[-1];
+ *d = d[-1];
+ ++d;
}
}
--d;
--
Dan Riley dsr@mail.lns.cornell.edu
Wilson Lab, Cornell University <URL:http://www.lns.cornell.edu/~dsr/>
"A new life awaits you in the off-world colonies"
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- cvs 1.12 expand_path variable expansion bug,
Dan Riley <=