Hum... I don't rememeber of this code... Where it is ?

(copying pike-devel, as I'm thinking there are potential pike issues at play here...)

This code is from caudium.c; make_tag_attributes() and encode_mapping(). But I'm not convinced it's directly that code. Any arrays I created and passed to f_replace() cause the crash. If you create the arrays, push them onto the stack, skip calling f_replace() then pop them back off, the problem goes away, without memory leak. I also don't get the problem if I pass all strings to f_replace(), but that's a different set of  replace code anyhow. There's a bunch of string builder code in f_replace, and string builder is also where things die later, so it makes me think there's something bad happening in there, though I realize that looks can be deceiving.

For those in pike-devel land that might have a moment to comment, the code in question seems to be:

static struct mapping *encode_mapping(struct mapping *mapping2encode, int encode_type)
{
  struct array            *indices, *values;
  struct mapping          *result;
  struct pike_string      *key = NULL, *val = NULL, *tmp = NULL;
  int                     i, j, k, do_replace;
  int                     size;

  indices = mapping_indices(mapping2encode);
  values = mapping_values(mapping2encode);
  size = (unsigned)indices->size;
  result = allocate_mapping(size);
  if(result == NULL)
    Pike_error("Can't allocate result mapping\n");

  /* encode any key/value pair in the mapping */
  for(i = 0; i < size; i++)
  {
    if(indices->real_item[i].type != T_STRING
         || values->real_item[i].type != T_STRING)
        continue;
    for(j = 0; j < 2; j++)
    {
      if(j == 0)
        tmp = indices->real_item[i].u.string;
      if(j == 1)
        tmp = values->real_item[i].u.string;
      /* let's see whether we have anything to encode */
       do_replace = 0;
       if(encode_type == 1)
         for (k = 0; k < XML_UNSAFECHARS_SIZE; k++) {
           if (memchr(tmp->str, xml_unsafechars[k][0], tmp->len)) {
             do_replace = 1;
             break;
           }
         }
       if(encode_type == 0)
         for (k = 0; k < HTML_UNSAFECHARS_SIZE; k++) {
           if (memchr(tmp->str, html_unsafechars[k][0], tmp->len)) {
             do_replace = 1;
             break;
           }
         }

       if (do_replace) {
         push_string(tmp);
         if(encode_type == 1)
         {
  push_text("<");
  push_text(">");
  push_text("&");
  push_text("\"");
  push_text("\'");
  push_text("\000");
  f_aggregate(6);
  
  push_text("&lt;");
  push_text("&gt;");
  push_text("&amp;");
  push_text("&#34;");
  push_text("&#39;");
  push_text("&#0;");
  f_aggregate(6);
  
          }
         if(encode_type == 0)
         {
  push_text("\"");
  f_aggregate(1);
  push_text("&quot;");
   f_aggregate(1);
       
      
        }
pop_n_elems(2);
//         f_replace(3);
         if(j == 0)
           copy_shared_string(key, Pike_sp[-1].u.string);
         if(j == 1)
           copy_shared_string(val, Pike_sp[-1].u.string);
         pop_stack();
      }
      else 
      {  
        if(j == 0)
          copy_shared_string(key, tmp);
        if(j == 1)
          copy_shared_string(val, tmp);
      }  
    }
    mapping_string_insert_string(result, key, val);
  }
  free_array(indices);
  free_array(values);
  return result;
}


the current code that's causing problems is:

http://cvsweb.caudiumforge.net/cgi-bin/cvsweb.cgi/caudium/src/cmods/_Caudium/caudium.c?rev=1.160;content-type=text%2Fx-cvsweb-markup;cvsroot=caudium

Previously, the two arrays that would be passed to f_replace() were premade, and a copy was just passed in (though it's not clear that they needed to be copied). This all started when I went looking for a leak, and added the free_array() calls that you see in f_make_tag_attributes() and encode_mapping(). Apparently the code doesn't crash if you don't free those arrays (indices and values), and that's the original author wasn't freeing them, but I don't think it should be necessary to endure a memory leak just for the sake of the code not  crashing. I can run this code for hours in an endless loop and not have problems or leaks, but caudium will die on the first request, usually somewhere inside Parser.HTML... I posed a backtrace to the main list a few days ago, but it had me totally confused.

It might be worth mentioning that I had a problem with a similar end result (bus errors coming from malloc() by way of string builder code) with some code I was writing, but that didn't have anything to do with this caudium c module. Unfortunately, I can't produce that code because I didn't check it in anywhere, and resolved the problem by rewriting the pike code that seemed to trigger it. The similarity of the two results makes me think that perhaps there's a problem somewhere...

Any suggestions?

Bill