Thread: Why does this code crash half way through?

  1. #1
    Registered User
    Join Date
    Nov 2013
    Posts
    107

    Why does this code crash half way through?

    And any way to improve this?

    whats wrong with this code, I'm trying to parse a .js file and replace all the ";" with ";\n" i.e add a new line after each ";". So I load the file into a char[] buffer then assign a string to this contents of this buffer. Then loop char by char through using an iterator and check for a ";", if found use replace. So int i gets to about 85898 then crashes with unknown error, 'i' should reach about 175653. It does work up till it crashes. And, is this not a simpiler way to load a file into a buffer, there is in C.




    Code:
    #include <iostream>
    #include <fstream>
    #include <algorithm>
    #include <vector>
    #include <stdlib.h>
    
    
    int main()
    {
        int length;
        char * buffer;
        std::string str;
        std::ifstream myfile;
        std::ofstream myfile2;
    
    
        myfile.open ("c:\\c.js",std::ios::binary);
        myfile2.open ("c:\\c2.js", std::ios::out);
        myfile.seekg(0,std::ios::end);
        length = myfile.tellg();
        myfile.seekg(0,std::ios::beg);
        buffer = new char [length];
        myfile.read (buffer,length);
        str = buffer;
        delete[] buffer;
    
    
        std::string::iterator it;
        std::string str2;
        int i = 0;
    
    
        for ( it=str.begin(); it < str.end(); it++ )
        {
            str2 = *it;
            //std::cout << i << std::endl;
            if(str2 == ";")
            {            
                str.replace(i, 1, ";\n");
            }
            i++;
        }
    
    
        myfile2 << str;
        myfile.close();
        myfile2.close();
        return 0;
    }

  2. #2
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    std::string's assignment operator assumes a zero-terminated string, and your code is not supplying the zero terminator. The assignment "str = buffer" (and anything subsequent) therefore has undefined behaviour - most likely in the form of running off the end of arrays (either your buffer, or the memory used internally by std::string, or both).

    I also fail to see the need to load a whole file in memory and then search for semi-colons, given your code is (attempting to) leave everything else untouched. Why not use character oriented input and, every time a ';' is received, output a '\n' immediately afterward?
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  3. #3
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    Replacing a single character with 2 or even 3 characters will invalidate the iterator.
    Try string::find in a loop.
    Also the allocated buffer has no space for a terminating 0 that you fail to append.

    Kurt

  4. #4
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,616
    I don't think you need to replace() anything. Just loop
    lastKnownSemicolon = s.find_first_of(';', lastKnownSemicolon);
    and then
    s.insert(s.begin() + lastKnownSemicolon + 1, '\n');

  5. #5
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    std::ifstream myfile("c:\\c.js",std::ios::binary); // <--- Utilize constructors. It means less code and it's often more efficient.
    std::ofstream myfile2("c:\\c2.js", std::ios::out);
    myfile.seekg(0,std::ios::end);
    auto length = myfile.tellg(); // <--- Avoid declaring files at the beginning of a function; declare them near where they are first used. Also, tellg() does not return an int. Use "auto" or std::size_t if your compiler does not support C++11.
    myfile.seekg(0,std::ios::beg);
    std::vector<char> buffer(length); // <--- No manual memory management. No need to free. Safe if something throws an exception.
    myfile.read (&buffer[0], buffer.size()); // <--- size() always returns the size of the buffer correctly. You can't go wrong here.
    std::string str(buffer.begin(), buffer.end()); // <-- Will automatically copy over all the data in the buffer in a safe way.
    //delete[] buffer; <--- No need for this any longer.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  6. #6
    Registered User
    Join Date
    Oct 2006
    Posts
    3,445
    is there any reason why you have to do all the char buffers? why can't you just read a character at a time and write to the destination file? it would be much simpler, and the buffered nature of the istream/ostream objects will help to keep performance up.
    What can this strange device be?
    When I touch it, it gives forth a sound
    It's got wires that vibrate and give music
    What can this thing be that I found?

  7. #7
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    To my experience, the "buffered nature" of iostreams is vastly slower than just directly reading and writing the entire file at once. If you have the memory to do so, of course. Your nature may vary.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  8. #8
    Registered User
    Join Date
    Oct 2006
    Posts
    3,445
    Quote Originally Posted by Elysia View Post
    To my experience, the "buffered nature" of iostreams is vastly slower than just directly reading and writing the entire file at once. If you have the memory to do so, of course. Your nature may vary.
    I suppose that's fair, but when we're talking about javascript files of a few hundred kB at most, it's unlikely that a simple operation like this would take more than a second or two on a modern computer, even using unbuffered i/o, especially when the OS does a lot of chaching and buffering of its own, unless you're doing something silly like calling fsync after every write. my point was that simpler is usually better.
    What can this strange device be?
    When I touch it, it gives forth a sound
    It's got wires that vibrate and give music
    What can this thing be that I found?

  9. #9
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    You are absolutely correct. You only really need to worry about these things if your program processes large files and you find it is slow.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Monitor displaying half good / half bad
    By Hiyo in forum Tech Board
    Replies: 3
    Last Post: 12-16-2006, 10:06 PM
  2. why does my code crash
    By Anddos in forum C++ Programming
    Replies: 5
    Last Post: 01-18-2006, 01:26 PM
  3. Why this code crash ?
    By MaaSTaaR in forum C Programming
    Replies: 4
    Last Post: 08-22-2005, 10:50 AM
  4. Half Humanoid, half Chimpanzoid
    By Zewu in forum A Brief History of Cprogramming.com
    Replies: 8
    Last Post: 06-19-2004, 04:23 PM
  5. Half life 2 source code stolen
    By joeyzt in forum A Brief History of Cprogramming.com
    Replies: 90
    Last Post: 10-18-2003, 08:59 PM