I am getting extra garbage at the end of the string returned by this function. Why?

char *read(char *fname)
{
    FILE *file=fopen(fname,"r");
    printf("Opening input file...\n");
    if (file==NULL)
        return NULL;
    int length=0;
    char *ret=new char[length];
    while (!feof(file))
    {
        char ch=fgetc(file);
        char *temp=new char[length+1];
        for (int i=0; i<length; i++)
            temp[i]=ret[i];
        delete[]ret;
        if (ch==EOF)
            ch=0;
        temp[length++]=ch;
        ret=temp;
    }
    fclose(file);
    return ret;
}

Dani AI

Generated

This is a classic case of heap corruption showing up far from the real mistake. The symptom — code runs under the debugger but crashes when run normally — almost always means some earlier buffer write stomped memory (debug builds and debuggers change heap layout and fill memory, which can hide the overwrite). called this out; was also right to be suspicious about out-of-bounds access. The actual failure in the write loop is a secondary crash caused by corruption elsewhere (the later posts point to the under-sized sprintf buffer in the highlight code).

Key points to fix and watch for

  • Undefined behavior (writing past an allocated buffer, using char to hold EOF, or using feof() as a loop condition) will behave differently under a debugger.
  • Avoid hand-calculating sizes for sprintf unless the math is rock-solid; one wrong count stomps the heap.
  • Prefer higher-level, safer types and APIs to eliminate most of these bugs.

Practical replacements (safe, simple)

  • Read an entire file into a std::string instead of manual new[]/loops:
    std::string readFile(const char* fname) {
      std::ifstream in(fname, std::ios::binary);
      if (!in) return {};
      std::ostringstream ss;
      ss << in.rdbuf();
      return ss.str();
    }
  • Build formatted strings with std::string (reserve capacity) instead of sprintf:
    std::string html;
    html.reserve(text.size() + 200);
    html += "<table width=\"100%\" border=\"0\" cellspacing=\"0\">\n";
    html += "<tr>\n<td>1.</td>\n<td bgcolor=\"#FFFFFF\">";
    html += text;
    html += "</td>\n</tr>\n</table>";

Debugging tips

  • Run with AddressSanitizer/Valgrind (Linux) or the MSVC debug heap / Application Verifier (Windows) to pinpoint the stomping allocation.
  • Rebuild with full debug info and run the release binary under the debugger to get a stack trace at the crash.
    Fix the overflow in the highlight/formatting code first — the rest should stop failing once the heap is no longer corrupted.

Recommended Answers

All 8 Replies

Let me explain a little further. In my debugger this all works fine, but when I actually run the program outside the debugger it fails... but why?

Sorry, my ideas were not gathered. Let me rephrase, I have a crash in this function but only when I run it outside the debugger. I get to the commented line before failure:

void write(char *text, char *fname)
{
    if (text==NULL)
        return;
    FILE *file=fopen(fname,"w");
    printf("Opening output file...\n");
    if (file==NULL)
        return;
    printf("Printing to file...\n");
    for (int i=0; text[i]; i++)
    {
        fputc(text[i],file);//failure here... but why?!
        printf("%c",text[i]);
    }
    printf("Done printing to file...\n");
    fclose(file);
}

i must be going out of bounds for the array text.

In your first code, why are you using new to get temp then you delete ret?

The code for read() works fine, its write() that is only working in the debugger and not outside of it. When I debug the program executes with no problem and with the expected output, but when I just run it outside the debugger it crashes and a windows error report screen pops up. Why does my write() function fail only outside the debugger?

I attached a word document that better explains using pictures. This has me stumped, I cannot think of any reason why the debugger can run the code successfully but outside it the code fails.

It seems to me that this piece of code is suspicious, in the "highlight" function, towards the end, you have:

char *temp=new char[len(text)+108];
    sprintf(temp,"<table width=\"100%%\" border=\"0\" cellspacing=\"0\">\n<tr>\n<td>1.</td>\n<td bgcolor=\"#FFFFFF\">%s</span></td>\n</tr>\n</table>",text);

I counted 116 additional characters (considering the newlines to be one character, which is not the case on all OSes). You should be using strcat instead.

I highly, highly recommend that you use std::string instead of C-style strings. I mean, all the code you have would be a heck of a lot more robust, faster and simpler if you used std::string instead.

Usually when you have code that works fine in debug mode and crashes or misbehaves in normal or release mode, it usually means that you have a memory corruption problem. Usually, debug-runs have additional code that fills any uninitialized memory with dummy values (like 0 or some hex pattern like 0xBAADF00D ). So, if your code happens to work fine in debug mode it is usually due to accessing memory that is either uninitialized or unused or used by something else and happens to have been zeroed, when in release mode that is no longer the case and your program crashes. In other words, the main thing that changes between debug and release execution modes is the resulting memory pattern and "empty" values, which should not have any effect on your code as long as you are not accessing memory that you shouldn't access and that you are not using uninitialized memory. In your case, and by seeing your code, I suspect you are doing both, and using std::string instead could pretty much insure that your do neither.

Ok, I changed those lines to:

char *temp=new char[len(text)+len("<table width=\"100%%\" border=\"0\" cellspacing=\"0\">\n<tr>\n<td>1.</td>\n<td bgcolor=\"#FFFFFF\">")+len("</span></td>\n</tr>\n</table>")];
    sprintf(temp,"<table width=\"100%%\" border=\"0\" cellspacing=\"0\">\n<tr>\n<td>1.</td>\n<td bgcolor=\"#FFFFFF\">%s</span></td>\n</tr>\n</table>",text);

just for a temporary fix. I am now working on converting it to std::string. I guess I just thought that using c-strings would be faster since I don't need all the functionality of an std::string. I guess I was wrong. Thanks!

>>I guess I just thought that using c-strings would be faster since I don't need all the functionality of an std::string. I guess I was wrong. Thanks!

In C++, the default choice should always be std::string , it will always be easier than C-style strings and usually faster in execution time too (or the same, depending on your skill at writing the C-style version of the code). C-style strings only appear in special circumstances (like interfacing with a C library). Then there are more advanced applications where std::string isn't sufficient and where more feature-rich tools are required (anything from std::stringstream , to boost::tokenizer , to boost::spirit ). But, the default is always std::string , because if you don't need all its features, the compiler won't make you pay a performance penalty (or additional code) for the features you don't use, so don't be afraid to use it even in the simplest use-cases.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.