Like Tree2Likes
  • 1 Post By rags_to_riches
  • 1 Post By adeyblue

Heap Corruption and Access Violation

This is a discussion on Heap Corruption and Access Violation within the C Programming forums, part of the General Programming Boards category; cA5.c Code: #include <stdio.h> #include <stdlib.h> #include <cA5_proto.h> #include <Windows.h> int main(int argc, char *argv[]) { char *filename = argv[1]; ...

  1. #1
    Registered User
    Join Date
    Nov 2012
    Posts
    5

    Thumbs up Heap Corruption and Access Violation

    cA5.c
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <cA5_proto.h>
    #include <Windows.h>
    
    
    int main(int argc, char *argv[])
    {
    char *filename = argv[1];
    WIN32_FIND_DATA filedata = {0};
    int fileSize = 0;
    FILE *fp = NULL;
    unsigned char *buffer = 0;
    
        //Checking for proper syntax
        if(argc != 2)
        {
            printf("Syntax: cA5.exe \"Filename\"\n", argv[0]);//Syntax error message
            return;
        }
    
    
        printf("Target file is %s\n", argv[1]);//Acknowledges user input by displaying intended file
    
    
        fileSize = getSmallFileLength(filename);//Calling function to populate WIN32_FIND_DATA struct
    
    
        buffer = (unsigned char *)malloc(filedata.nFileSizeLow);//Allocating memory to buffer 
        if(buffer == NULL)
        {
            printf("There was an error allocating memory for file contents!\n");//Error message
        }
    
    
        readWriteFileContents(fp, buffer, fileSize, argv[1]);
    
    
        free(buffer);//Freeing up the allocated memory
    }
    
    
    /* 
    *  FUNCTION      : readWriteFileContents
    *
    *  DESCRIPTION   : Reads the file contents then writes it to the file 'contents.txt'
    *
    *  PARAMETERS    : char *filename     : pointer to argv[1] "Name of intended file on command line"
    *                   FILE *fp             : uses the file pointer used to open the file                       
    *                   int fileSize      : lets the writing know when to stop the process
    *
    *  RETURNS       : 0                 : returns to main menu
    */
    int readWriteFileContents(FILE *fp, unsigned char *buffer, int fileSize, char *filename)
    {
    int i = 0;
        
        fp = fopen(filename, "r");
        if(fp == NULL) 
        {
            printf("There was an error found!\n");//Error message
            return 1;
        }
        else
        {
            fread(buffer, 1, fileSize, fp);
            
            fclose(fp);
            
            fp = fopen("contents.txt", "w");
            if(fp == NULL)
            {
                printf("There was an error opening contents.txt! Make sure it's in the current directory!");//Error message
                return 1;
            }
            else
            {
                while(i <= fileSize)//Runs fprintf until it reachs the end of the file
                {
                    if((i % 10) == 0)
                    {
                        fprintf(fp, "\n");
                        i++;
                    }
                    else
                    {
                        fprintf(fp, "%03d ", buffer[i]);
                        i++;
                    }
                }
                fclose(fp);
                return 0;
            }
            return 0;
        }
        return 0;
    }
    fileUtilities.c
    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include "cA5_proto.h"
    #include <Windows.h>
    
    /* 
    *  FUNCTION      : getSmallFileLength
    *
    *  DESCRIPTION   : Used to populate the WIN32_FIND_DATA Struct, giving us the required
    *                 : nFileSizeLow value
    *
    *  PARAMETERS    : const char *filename     : pointer to argv[1] "Name of intended file on command line"
    *
    *  RETURNS       : nFileSizeLow             : returns to main menu
    */
    int getSmallFileLength(const char *filename)
    {
        WIN32_FIND_DATA filedata = {0};
        HANDLE hFind;
    
    
        //Function only works for files below 2,147,483,647 bytes, limitation in function
        FindFirstFile(filename, &filedata);//Populates WIN32_FIND_DATA struct according to proper file name
        
        if(hFind == INVALID_HANDLE_VALUE) //Error message
        {
            printf("There was an error!");
            return -1;
        }
        else if(filedata.nFileSizeLow < 1)
        {
            return 0;    
        }
    
    
        return filedata.nFileSizeLow;
    
    
        FindClose(hFind);
    }
    Getting a heap corruption problem at the fread(); line in the second module function in cA5.c.

    Basically this program takes in a file, finds the nFileSizeLow value and reports it back to the main function. When that is done, it takes the file and allocates the proper amount of memory depending on the file size. It then takes in the file bytes then spews them out into a new file called 'contents.txt' in a nicely formatted package.

    Everything goes well until it gets to the portion before writing the contents out to the new file. (Around line 60)

    cA5_proto.h just includes the prototypes for the functions in this project
    Last edited by Mawler; 11-28-2012 at 07:56 PM.

  2. #2
    a_capitalist_story
    Join Date
    Dec 2007
    Posts
    2,650
    What exactly do you think is the value of the argument passed here?

    Code:
    buffer = (unsigned char *)malloc(filedata.nFileSizeLow);//Allocating memory to buffer
    If you're not sure, print it. Are you sure you weren't looking to use a different variable there?

    Oh, and if you're writing and compiling this as C code, you should not cast the return value of malloc. And having code after a return in a function should be throwing a warning, so if it's not--turn up your warning level. If it is, stop ignoring the warnings and fix them!
    Salem likes this.

  3. #3
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,304
    Ya know, just a single return 0; at the end of readWriteFileContents would suffice, you don't need three of them.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #4
    Registered User
    Join Date
    Nov 2012
    Posts
    5
    I had noticed that filedata.nFileSizeLow was not being populated while I was watching it during debugging in Visual Studio 2010. Changed it to:
    Code:
    buffer = (unsigned char*)malloc(fileSize);//Allocating memory to buffer
    Trimmed down the return values in readWriteFileContents to 1. Thank you both for the input. I knew it was something wrong with my malloc(); line. Just wasn't too sure!

  5. #5
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,551
    I see you didn't remove the (unsigned char*) cast as advised.
    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.

  6. #6
    'Allo, 'Allo, Allo
    Join Date
    Apr 2008
    Posts
    611
    And since nobody mentioned it, getSmallFileLength has a couple of errors and a leak.

    Code:
        HANDLE hFind;
        //Function only works for files below 2,147,483,647 bytes, limitation in function
        FindFirstFile(filename, &filedata);//Populates WIN32_FIND_DATA struct according to proper file name
        
        if(hFind == INVALID_HANDLE_VALUE) //Error message
    A) You never assign the result of FindFirstFile to hFind (nor initialize it), so the chances of the function signalling error are random.
    B) You never assign the result of FindFirstFile to hFind (or anything for that matter), so there's no way you can release the resources with FindClose.
    C) FindClose is after the return, so even if you did the assignation, it wouldn't matter anyway
    D) Since you're not actually doing any finding of files, replace the Find-y bits of your function with GetFileAttributesEx(fileName, GetFileExInfoStandard, &fileData) and change the type of fileData to WIN32_FILE_ATTRIBUTE_DATA and you don't have to assign or cleanup anything!
    Last edited by adeyblue; 11-29-2012 at 09:51 AM.
    AndiPersti likes this.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Heap Corruption Error help
    By shivam1992 in forum C Programming
    Replies: 4
    Last Post: 11-22-2011, 05:27 PM
  2. This may be due to a corruption of the heap
    By Cobs in forum C++ Programming
    Replies: 3
    Last Post: 01-13-2011, 08:53 PM
  3. Possible Heap Corruption
    By Michael5978 in forum C++ Programming
    Replies: 2
    Last Post: 04-06-2010, 04:09 PM
  4. This may be due to a corruption of the heap
    By krishnampkkm in forum C++ Programming
    Replies: 4
    Last Post: 06-26-2009, 03:19 AM
  5. DLL & free() Heap Access Violation
    By kuphryn in forum Windows Programming
    Replies: 0
    Last Post: 08-13-2002, 10:45 PM

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