Quite right, Ken. The new patch hopefully takes care of that issue, and
fixes an additional bug in the concat operator.
Ken Fox wrote:
> Jeff wrote:
> > diff -ru parrot/core.ops parrot_orig/core.ops
> > --- parrot/core.ops Tue Nov 6 11:14:25 2001
> > +++ parrot_orig/core.ops Wed Nov 7 20:53:05 2001
> > @@ -199,7 +199,7 @@
> > string_destroy($1);
> > tmp = malloc(len + 1);
> > read($2, tmp, len);
> > - s = string_make(interpreter, tmp, len, 0, 0, 0);
> > + s = string_make(interpreter, tmp, strlen(tmp), 0, 0, 0);
> > $1 = s;
> > free(tmp);
> > }
>
> You just called strlen on a random chunk of binary data.
> How do you know it's null terminated?
>
> Shouldn't the code really be:
>
> if ((len = read($2, tmp, len)) >= 0) {
> s = string_make(interpreter, tmp, len, 0, 0, 0);
> }
> else {
> /* handle error? */
> }
> $1 = s;
> free(tmp);
>
> - Ken
--
--Jeff
<[EMAIL PROTECTED]>
diff -ru parrot/core.ops parrot_orig/core.ops
--- parrot/core.ops Tue Nov 6 11:14:25 2001
+++ parrot_orig/core.ops Wed Nov 7 23:56:07 2001
@@ -196,12 +196,31 @@
STRING *s;
INTVAL len = $3;
- string_destroy($1);
+ s = $1;
+
tmp = malloc(len + 1);
- read($2, tmp, len);
- s = string_make(interpreter, tmp, len, 0, 0, 0);
- $1 = s;
- free(tmp);
+ if(read($2,tmp,len)==0) {
+ free(tmp); /* Clear up the potential memory leak */
+ if(s && s->bufstart != NULL) {
+ free(s->bufstart); /* Free the old allocated string. */
+ }
+ s->bufstart = NULL;
+ s->buflen = 0;
+ string_compute_strlen(s);
+ s->strlen = 0;
+ }
+ else {
+ if(s && s->bufstart != NULL) {
+ free(s->bufstart);
+ s->bufstart = tmp;
+ s->buflen = strlen(tmp);
+ string_compute_strlen(s);
+ }
+ else {
+ $1 = string_make(interpreter, tmp, strlen(tmp), 0, 0, 0);
+ free(tmp);
+ }
+ }
}
@@ -1058,7 +1077,11 @@
=cut
AUTO_OP concat(s, s|sc) {
- $1 = string_concat(interpreter, $1, $2, 1);
+ STRING *s1 = $1;
+ STRING *s2 = $2;
+ if(string_length($2)>0) {
+ $1 = string_concat(interpreter, s1, s2, 1);
+ }
}
AUTO_OP concat(s, s|sc, s|sc) {
