Memory issue with new[] and delete[]

This is a discussion on Memory issue with new[] and delete[] within the C++ Programming forums, part of the General Programming Boards category; That's great, but now I can't delete temp. I need to go through a file and parse it out. If ...

  1. #16
    Registered User
    Join Date
    Jul 2004
    Posts
    27
    That's great, but now I can't delete temp.

    I need to go through a file and parse it out. If I go over 26 in my increases in the pointer, I can't delete it

    Soo, basically...
    Code:
    file_buffer += 26;
    delete[] file_buffer; //Those work fine, but the below two don't
    
    file_buffer += 27;
    delete[] file_buffer;
    Dynamic memory sucks

  2. #17
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    >That's great, but now I can't delete temp.

    You shouldn't delete temp_buffer, only file_buffer. They both point to the same memory. temp_buffer just points to a location past file_buffer.

    The basic rule is you need one delete for every new.

  3. #18
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,422
    > Segmentation fault yet again...
    Then your problem is somewhere else
    Once you've trashed your memory, then any future use will also likely fail, no matter how correct it seems.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  4. #19
    Registered User
    Join Date
    Jul 2004
    Posts
    27
    Err, yeah.. Whups.. Dunno why I tried to delete temp o_0;

    So basically if I am playing with a dynamically allocated string, it's best to make a pointer and play with that pointer instead... ok, Thanks alot =)

  5. #20
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    >So basically if I am playing with a dynamically allocated string, it's best to make a pointer and play with that pointer instead
    That's right. If you are going to be calling any function where the pointer gets changed, or incrementing the pointer, make a copy. Otherwise you've lost the address where "new" allocated it.

    Now you can pass the pointer into a function, and modify it there, because a temporary copy is passed to the function. So:

    Code:
    void skip_lines(char *file_buffer)
    {
      file_buffer = strchr(file_buffer, '\n') + 1;
      file_buffer = strchr(file_buffer, '\n') + 1;
      //
      // Do some processing with file_buffer here
      //
    }
    Then in main(), file_buffer would still point to the original, so:
    delete []file_buffer;
    would work.
    Last edited by swoopy; 08-06-2004 at 11:47 AM.

  6. #21
    Registered User
    Join Date
    Jul 2004
    Posts
    27
    Argh, Got so far and now memory comes back to haunt me. Theres a lot more to this, though.

    A segmentation fault occurs in my script if there are more than 2 lines in the configuration file that occupy space (As in it's not just a new line).
    I traced this segmentation fault to the following line:
    Code:
    while ((destination_directory = config.return_dest()) != NULL) {
    Here is the code for inside the while loop:
    Code:
    while ((destination_directory = config.return_dest()) != NULL) {
          if (dirent->d_type == DT_DIR) {
            // Open up the destination file that matches this one. Start opening as a directory.
            destination_name = new char[strlen(destination_directory) + strlen(dirent->d_name) + 2];
            strcpy(destination_name, destination_directory);
            if (source[strlen(source)] != '/')
              strcat(destination_name, "/");
            strcat(destination_name, dirent->d_name);
            if ((d_directory = opendir(destination_name)) == NULL) {
              // Didn't open. Find out why and see if we can work with it!
    	  if (errno == ENOTDIR || ENOENT) {
    	    options.print_output("Directory \"",0);
    	    options.print_output(destination_name, 0);
    	    options.print_output("\" does not exist. Attempting to create...\n", 0);
    	    if (mkdir (destination_name, 0777) == -1) {
    	      options.print_output("Unable to create destination directory \"",0);
    	      options.print_output(destination_name, 1);
    	      options.print_output("\"\n", 0);
    	      delete[] destination_name;
    	      continue;
    	    }
      	  } else {
    	    options.print_output("Unable to read destination directory \"",0);
    	    options.print_output(destination_name, 1);
    	    options.print_output("\"\n", 0);
     	    delete[] destination_name;
    	    continue;
    	  }
            }
    	source_name = new char[strlen(source) + strlen(dirent->d_name) + 2];
    	if (source_name[strlen(source)] != '/')
              strcat(source_name, "/");
            strcat(source_name, dirent->d_name);
    	copy(source_name, destination_name);
    	delete[] source_name;
    	delete[] destination_name;
    	continue;
          }
    Here is the code to the config.return_dest() function:
    Code:
    char* config::return_dest () {
      if (dest_loop >= dest_num) {
        dest_loop = 0;
        return NULL;
      }
      dest_loop++;
      return dest[dest_loop - 1];
    }
    I tried to figure out what was causing the segmentation fault, but couldn't see any error there... Then all of a sudden my cat jumped up on my keyboard, and killed my terminal window. So I went back in, and just ran the program from absolute path instead of just typing "bin/sync". Amazingly, no segmentation fault! Huh, lets try it from relative! Works again... So I guess I've isolated the issue to this small chunk of code, but I can't seem to get it working right:
    Code:
            char    *env = getenv( "PATH" );
            if ( env != NULL ) {
                char buff[PATH_MAX];
                int  n;
                while ( *env &&
                        sscanf( env, "%[^:]%n", buff, &n ) == 1 ) {
                    FILE *fp;
                    buff[n] = '\0';
                    strcat( buff, "/" );
                    strcat( buff, filename );
                    if ( (fp=fopen(buff,"r")) != NULL ) {
    
    		   buff[strlen(buff) - strlen(basename(buff))] = '\0';
    		   strcat (buff, "../conf/sync.conf");
                       file_id = new char[strlen(buff) + 1];
     	           strcpy(file_id, buff);
    		   file = file_id;
    		   fclose(fp);
                       break;
                    }
                    env += n;
                    if ( *env == ':' ) env++;
                }
          }
    I also tried removing this particular code and setting a static value to see if it would cause a problem:
    Code:
    file = new char[strlen("./conf/sync.conf") + 1];
    strcpy(file, "./conf/sync.conf");
    No problems at all.

    Thanks for the help, if you took the time to look at all that O_o;
    Last edited by Salem; 08-06-2004 at 04:20 PM. Reason: Code formatting

  7. #22
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,422
    I think you've dug such a big hole in your memory that it's impossible to tell where (and how many) problems you have.

    You seem to have little idea of how strings are stored, so my guess is memory problems are rampant in your code. You always just post the bit which is falling over at the moment without really getting to the real cause of the problems. Until you grasp the idea that memory problems have "a cause" (where the bug is) and "an effect" (where it segfaults), and that these two events can be separated by large amounts of code and time, there is not a lot we can do to help.

    For example
    source[strlen(source)]
    is always the \0 character at the end of the string, so comparing it against anything else will always fail.

    And this
    if (errno == ENOTDIR || ENOENT) {
    is always true, because you probably meant this
    if (errno == ENOTDIR || errno == ENOENT) {

    Plus you seem to have generated an awful lot of code without any real testing, so any previous memory errors have gone undetected until now.


    Personally, I'd suggest you
    a) start again
    b) use the std::string to manage all your strings for you and let it worry about all the dynamic aspects.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  8. #23
    Registered User
    Join Date
    Jul 2004
    Posts
    27
    I do have a grasp on how memory works :/
    Some parts I don't, sure. But I do create strings, I make pointers to them if I manipulate the code a lot, and I delete them when they are done.

    I do find the issues by doing fprintf's to stderr for til I find the problem, then correct it....

    I'm new to the memory thing, I know. The reason I was posting just bits of code is because I found how to cause a segmentation fault shortly after creating the variable, and that's how I learned that. I have no real books on this stuff, and just go from what I find. I should get a book :P

    I don't think there are any memory problems in there. I always allowcate enough memory for things, if not extra o_0;

  9. #24
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    In addition to the bugs Salem pointed out, there's also this:
    Code:
    >	source_name = new char[strlen(source) + strlen(dirent->d_name) + 2];
    
    At this point nothing has even been copied into source_name, so the if() below won't work.
    
    >	if (source_name[strlen(source)] != '/')
    >          strcat(source_name, "/");

  10. #25
    Registered User
    Join Date
    Jul 2004
    Posts
    27
    Salem>Plus you seem to have generated an awful lot of code without any real testing, so any previous memory errors have gone undetected until now.

    Is there a way to test the new operator? I always did if ((var = (char*) malloc(size)) == NULL) testing, but now that I'm using new I didn't think that would work.. I'll try it =)


    >source[strlen(source)]
    is always the \0 character at the end of the string, so comparing it against anything else will always fail.

    I forgot to minus one for the array since it starts at 0 x_X thanks

    Didn't even notice that swoop as well, thanks ;P

    And I don't want to use the std thing... That requires iostream.h I think, eh?

    I'd much rather learn how to manipulate the strings fully before using the easy way x_X

Page 2 of 2 FirstFirst 12
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 14
    Last Post: 06-28-2006, 01:58 AM
  2. Replies: 8
    Last Post: 01-17-2006, 04:39 PM
  3. delete[] without new[]?
    By ChadJohnson in forum C++ Programming
    Replies: 6
    Last Post: 04-04-2005, 11:39 AM
  4. Unknown Memory Leak in Init() Function
    By CodeHacker in forum Windows Programming
    Replies: 3
    Last Post: 07-09-2004, 09:54 AM
  5. allocating memory screws up data being read in from file
    By Shadow12345 in forum C++ Programming
    Replies: 5
    Last Post: 12-06-2002, 02:23 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21