Segmentation fault

[Derek Martin]

On Mon, 7 Feb 2000, Jerry Feldman wrote:

> First, whenever free() crashes it is generally because the current or a 
> previous freed block has been stepped on. Every malloc is a bit different, 
> but when you get a malloc block, it normally contains a header containing a 
> couple of pointers (for returning it to the freelist) and a size, and maybe 
> a few more things. The pointer that is returned generally points to the 
> first byte following that. You also generally get more than you asked for 
> because the blocks are guaranteed to start on a "natural" boundary (32 bits 
> on a 32 bit machine, 64 on a 64 bit machine, or even 128 bits).

This makes sense, but I don't see how my code is doing that... I'll show
you what I mean in a sec.

> First, you do not request the correct len. In your case len == strlen. You 
> should add 1 to allow for the trailing null termination.
> >     temp = (char *)malloc( len * sizeof(char));

OH!  I just caught it.  The reason I allocated it that length is that
eventually temp is only going to hold len - 1 characters, but I changed
the code afterwards to prevent string from being mangled, and ended up
copying the whole thing into temp.  Oops. :)  I bet that will fix it.


> Secondly, in neather case do you check if temp is NULL. I would put a check 
> or an assertion in here. Remember that you are calling this recursively. 
> Your stack should be ok.
> eg; assert(temp);

Yep, I agree that I should check both malloc calls, but I was running this
on only my machine under known conditions... I watched it run (with top)
and the RSS stayed fixed at about 375k or so... so I didn't bother. It
became plainly obvious that I had originally forgotten to free() the temp
vars when I watched it run with top... hehehe

> Another thing I don't like is your rest_of_string function. Look at the 
> case when i == (len - 1); Remember that string[i] would point to the end of 
> the original string, so what you are doing is:
> string[len - 1] = '\0';
> strcat(string, string + len);
> Since string[len] contains garbage, you are possibly coipying beyond the 
> end of the allocated block into another block's header.

I definitely don't agree here.  If you look more closely you'll see that
even in the case of ( i == ( len - 1 )) that essentially does:

  strcat( "stuf\0", "\0" );

{assuming that the original string was "stuff" and assuming that I
didn't already overrun it, of course...}

which is perfectly valid, and precisely what is needed.  But I'll go back
and fix the other error when I get the chance, and I bet that will solve
the problem.  Someone else suggested this too. Thanks both for your help.
:)
 
Though I still don't quite understand why this works fine if the string's
length is less than 12, and not otherwise... 

> > char *
> > rest_of_string( char *string, int usedchar )
> > {
> >   
> >   string[usedchar] = '\0';
> >   strcat( string, (string + usedchar + 1) );
> >   return string;
> >   
> > }


-- 
"Quis custodiet ipsos custodes?"    "Who watches the watchmen?" 
-Juvenal, Satires, VI, 347 

Derek D. Martin      |  Senior UNIX Systems/Network Administrator
Arris Interactive    |  A Nortel Company
derekm at mediaone.net  |  dmartin at ne.arris-i.com
-------------------------------------------------


-
Subcription/unsubscription/info requests: send e-mail with
"subscribe", "unsubscribe", or "info" on the first line of the
message body to discuss-request at blu.org (Subject line is ignored).