Thread: free() - program crash

  1. #1
    Registered User
    Join Date
    Feb 2014
    Posts
    4

    free() - program crash

    Hi

    I have a problem with using free() which causes a "Segmentation Fault".
    What I am trying to do is:
    1) Save string from the struct into maloced memory
    2) Save the size of each string (incl. \0)

    Later not every string from the struct should be saved but for testing I made it easier. Also the strings in partlist are assigned to the struct by a file later.
    The problem occurs when num reaches an amount of 7. I have marked the positon where the error occurs in the code.

    OS: Unix/Linux

    So here is my code:
    Code:
    #include <stdio.h>
    #include <string.h>
    #include <unistd.h>
    #include <stdlib.h>
    
    typedef struct{
        char kind[30];
        char ident[30];
        char desc[100];
        char detail[100];
        char distri[30];
        char ordnr[30];
        char price[4];
        char num[4];
        char mount[30];
        char pdf[100];
    
    } PARTS;
    
    int main(int args, char* argv[])
    {
        PARTS partlist[13];
    
        strcpy( partlist[0].kind , "1st" );
        strcpy( partlist[1].kind , "2nd" );
        strcpy( partlist[2].kind , "3rd" );
        strcpy( partlist[3].kind , "4th" );
        strcpy( partlist[4].kind , "5th" );
        strcpy( partlist[5].kind , "6th" );
        strcpy( partlist[6].kind , "7th" );
        strcpy( partlist[7].kind , "8th" );
        strcpy( partlist[8].kind , "9th" );
        strcpy( partlist[9].kind , "10th" );
        strcpy( partlist[10].kind , "11th" );
        strcpy( partlist[11].kind , "12th" );
        strcpy( partlist[12].kind , "13th" );
    
        int i;
        int n;
        int num = 1;
    
        char *kind = (char *) malloc( strlen( partlist[0].kind ) +1 );
        int *size = (int*) malloc( 1 );
    
        strcpy( kind , partlist[0].kind );
        *size = strlen( partlist[0].kind ) +1;
    
        for( i=1 ; i<13 ; i++ ){
            //if( ... ){ //will be implemented later but is the reason for using num
                int mal_size = 0;
                num++;
    
                printf("%d.1\tnum: %d\n",i ,num);
    
                int *size_tmp = (int *) malloc( num );
                if( size_tmp == NULL )
                    printf("size_tmp = NULL!\n");
    
                for( n=0 ; n<num-1 ; n++)
                    size_tmp[n] = size[n];
                size_tmp[num-1] = strlen( partlist[i].kind ) +1;
    
                free( size );
                size = NULL;
                size = (int *) malloc( num );
    
                for( n=0 ; n<num ; n++ ){
                    size[n] = size_tmp[n];
                }
    
                free( size_tmp );//SEGMENTATION FAULT!!!!  @num=7
                size_tmp = NULL;
    
                printf("%d\n",num);
    
                for( n=0 ; n<num ; n++)
                    mal_size += size[n];
    
                char *kind_tmp = (char *) malloc( mal_size );
                memcpy( kind_tmp , kind , mal_size - size[num-1] );
                memcpy( kind_tmp + mal_size - size[num-1] , partlist[i].kind , size[num-1] );
    
                free( kind );
                kind = NULL;
                kind = (char *) malloc( mal_size );
    
                memcpy( kind , kind_tmp , mal_size );
                free( kind_tmp );
                kind_tmp = NULL;
            //}END OF if
        }
    
        free( kind );
        free( size );
    
        return( 0 );
    }
    I hope somebody could help me!

    Kind regards
    michael

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    This looks wrong:
    Code:
    int *size = (int*) malloc( 1 );
    It is unlikely that sizeof(int) == 1. Rather, to dynamically allocate space for one int, you should write:
    Code:
    int *size = malloc(sizeof(*size));
    Or in this case, it may be even better to write:
    Code:
    int *size = malloc(num * sizeof(*size));
    and indeed later you need to make a similiar correct to another malloc call.

    Without looking too closely at your code though, I wonder if the size should just be part of the PARTS struct.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Feb 2014
    Posts
    4
    Hi

    Thank you for you quick response.

    This code should later be added to another program. An array of PARTS will the contain information about electrictal components. The code shown above should then find all different strings stored in partlist[x].kind (not implemented yet but indicated with a comment). So in the end kind should contain all different kinds:
    for example:
    kind[30] = 1st\02nd\03rd\04th\05th\0 .....
    size should contain: size[0] = 4 ; size[1] = 4 ; size[2] = 4 ....

    LATER: a string that exists for example in partlist[0].kind AND in partlist[5].kind or more often should be added only once to kind and the length of it to size

    It's really hard to describe -.-

    So
    Code:
    int *size = (int*) malloc( 1 );
    should not be a problem and could also be written it as:
    Code:
    int *size = (int*) malloc( num );
    I hope I didn't misunderstand you and I also hope it is understanable what I want to do

    Kind regards
    michael

  4. #4
    Registered User
    Join Date
    Jun 2011
    Posts
    4,513
    malloc()

    void* malloc (size_t size);
    Allocates a block of size bytes of memory, returning a pointer to the beginning of the block.
    The standard dictates that the minimum size of an integer is 2 bytes (draft: 5.2.4.2.1).

    Code:
    int *size = (int*) malloc( 1 );
    So you're allocating one byte to store a value that is at least 2 bytes.

    Hence, you should heed the advise given by laserlight.

    Also, you don't need to cast "malloc()".
    Last edited by Matticus; 02-14-2014 at 09:58 AM. Reason: Correction from phantomotap - thanks!

  5. #5
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    The standard dictates that the minimum size of an integer is 4 bytes (draft: 5.2.4.2.1).
    O_o

    Misconception.

    The standard specifically allows `sizeof(char) == sizeof(short)', `sizeof(short) == sizeof(int)', `sizeof(int) == sizeof(long)', and `sizeof(long) == sizeof(long long)'.

    An `int' may be larger, but the standard only says that `int' is at least two bytes--same as `short'.

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  6. #6
    Registered User
    Join Date
    Jun 2011
    Posts
    4,513
    Oops, you're right - I did the bit-to-byte math wrong (guess I did bit-to-nibble for some reason by mistake). Thanks for the correction.

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by michi099
    This code should later be added to another program. An array of PARTS will the contain information about electrictal components. The code shown above should then find all different strings stored in partlist[x].kind (not implemented yet but indicated with a comment). So in the end kind should contain all different kinds:
    for example:
    kind[30] = 1st\02nd\03rd\04th\05th\0 .....
    size should contain: size[0] = 4 ; size[1] = 4 ; size[2] = 4 ....
    Ah, so kind will store a sequence of null terminated strings, and the length of the ith string will be given by size[i].

    This can work, but as Matticus cited in post #4, the argument provided to malloc is the number of bytes required, not the number of elements required. This is why I gave this example if you want num ints:
    Code:
    int *size = malloc(num * sizeof(*size));
    Incidentally, you may want num to be a size_t instead of an int.

    Quote Originally Posted by Matticus
    The standard dictates that the minimum size of an integer is 4 bytes
    No, it dictates the minimum range of an int. That minimum range, given an 8-bit byte, means that the minimum size of an int is 2 bytes, mathematically. However, a "byte" is not necessarily 8 bits (hence the existence of CHAR_BIT), so sizeof(int) == 1 is theoretically possible.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Registered User
    Join Date
    Feb 2014
    Posts
    4
    Hey Laserlight, hey Matticus

    Thank you for your explanation!
    You and Laserlight are right. I didn't thought about this because when I debuged the code the all storing numbers in size work without problems UNTIL num gets 7!? Because of this I did not even think about not getting an integer into one byte. What a shame :/

    I have correted it now to (as also shown by Laserligth --> last post)
    Code:
    size = (int *) malloc( num * sizeof( int ) );
    And it works like a charm!!
    THANK YOU VERY MUCH

    Do you have any idea why debugging works until num gets 7?
    I use Eclispe and for monitoring size i used: size[10] and every number was listed correct!? I am confused.

    Also, you don't need to cast "malloc()".
    LOL i did not know this! But in a tutorial i made it was shown so I didn't try it without a cast

    @Laserlight
    Yeah you got it. So I need not worry about my ability of expression

    Kind regards
    michael
    Last edited by michi099; 02-14-2014 at 09:52 AM.

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by michi099
    I have correted it now to
    Code:
    size = (int *) malloc( num * sizeof( int ) );
    That is fine, but I recommend removing the cast as it is unnecessary unless you intend your code to be compilable as C++. In the unfortunate event that you forget to include <stdlib.h>, the cast would likely hide the mistake, whereas without it your compiler is likely to warn you. I recommend using sizeof(*size) instead of sizeof(int) as it is immediately clear that it is correct, whereas it may be possible for the type of size to be changed under maintenance, yet the sizeof was left unchanged.

    Quote Originally Posted by michi099
    Do you have any idea why debugging works until num gets 7?
    I use Eclispe and for monitoring size i used: size[10] and every number was listed correct!? I am confused.
    As in why your program only crashed after a few iterations of the loop? It could be the case that for the first few iterations, the memory actually allocated was sufficient, but at some point that no longer held true, and you began accessing memory beyond the "extended" boundary. In any case, this is undefined behaviour.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  10. #10
    Registered User
    Join Date
    Feb 2014
    Posts
    4
    That is fine, but I recommend removing the cast as it is unnecessary unless you intend your code to be compilable as C++. In the unfortunate event that you forget to include <stdlib.h>, the cast would likely hide the mistake, whereas without it your compiler is likely to warn you. I recommend using sizeof(*size) instead of sizeof(int) as it is immediately clear that it is correct, whereas it may be possible for the type of size to be changed under maintenance, yet the sizeof was left unchanged.
    Thank you for the tips!

    As in why your program only crashed after a few iterations of the loop? It could be the case that for the first few iterations, the memory actually allocated was sufficient, but at some point that no longer held true, and you began accessing memory beyond the "extended" boundary. In any case, this is undefined behaviour.
    Thank you for your description! It is now clear to me.

    Now I have to thank you all who helped me solving the problem. I'm amazed of how fast you answered.
    You've brought my motivation back

    Have a nice day

    Kind regards
    michael

  11. #11
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    No, it dictates the minimum range of an int. That minimum range, given an 8-bit byte, means that the minimum size of an int is 2 bytes, mathematically. However, a "byte" is not necessarily 8 bits (hence the existence of CHAR_BIT), so sizeof(int) == 1 is theoretically possible.
    O_o

    [Edit]
    I compose responses outside of "Firefox"; somehow my reply went wonky.
    [/Edit]

    That is not entirely complete. The existence of an environment where `CHAR_BIT != n^2' for some integer `n' and `sizeof(char) == sizeof(int)' while maintaining conformance to the rest of the standard is so remote as to be absurd. Yeah, it is "theoretically possible", but it is "theoretically possible" in the same way that I might one day visit the moon.

    [Edit]
    The standard taken entirely, as a whole, prevents a practical `sizeof(char) == sizeof(int)' environment. The various "ctype.h" functions all take an `int', as you know. The standard describes the domain of these functions in terms of "any `unsigned char' value or `EOF'". Furthermore, the standard describes the `EOF' value as being a negative value--so being in a domain unique with respect to `unsigned char' values. You have to allow iterating over the valid range of any `unsigned char' value without the value ever being equivalent to `EOF' (You require `for(unsigned char c(0); true; ++c)' to never set `c' to `EOF'.) which requires a unique representation for the sentinel. For any `unsigned char' representation to possibly have a unique value with the same number of bits in the representation as a sentinel with `int', the environment would have to somehow provide a means of trapping the "sentinel" value or "clamping" the value to a reduced domain during normal operations. Such limitation on values is generally handled in hardware by using different operations--possibly `inc8' or `inc16'--for different representations, but with `sizeof(char) == sizeof(int)' the environment would need unique operations for the same representation.
    [/Edit]

    I'm not saying that it is impossible; it is however impractical. As was discussed in a recent thread, the expectation of `CHAR_BIT' as being anything other than 8 requires way more work than just a `typedef', and you are apparently thinking "powers of two" as in common hardware. If the `sizeof(char) == sizeof(int)' is a real consideration, you also have to consider cases where `CHAR_BIT == 12' or similar. (Just imagine trying to read an ASCII file produced on common environments!) As in that thread, the possibility is so remote that for normal code examining the environment and producing an error is more reasonable.

    [Edit]
    Again yes, the possibility is real. The point is, in reality, such an environment is not likely to conform to the standard in other ways. (For example, I've seen `CHAR_BIT == 9', but that environment didn't provide `malloc'/`free'.) If you are targeting such a unique platform, you can probably just assume "does not conform to standard C".
    [/Edit]

    Soma
    Last edited by phantomotap; 02-14-2014 at 11:29 AM.
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  12. #12
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    Quote Originally Posted by phantomotap View Post
    Quote Originally Posted by Matticus View Post
    The standard dictates that the minimum size of an integer is 2 bytes.
    Misconception.

    The standard specifically allows `sizeof(char) == sizeof(short)', `sizeof(short) == sizeof(int)', `sizeof(int) == sizeof(long)', and `sizeof(long) == sizeof(long long)'.
    Actually, the standard does not make any such specific allowance.

    Section 5.2.4.2.1 referred to be matticus says nothing at all about results produced by the sizeof() operator. It specifies ranges of values that the integral types must, at minimum, be able to represent. For example, the minimum allowed value of UCHAR_MAX (the implementation defined limit on maximum value that can be stored in an unsigned char) is 255.

    The only constraints stipulated on sizeof() is that sizeof gives gives the result 1 for the three char types (char, unsigned char, signed char), and gives the number of bytes making up other types.

    Nowhere in the standard is there any mention about possible relationships of results from sizeof for different integral types.
    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.

  13. #13
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    Actually, the standard does not make any such specific allowance.
    O_o

    The "variability" of `CHAR_BIT' and relationship of `sizeof' was already addressed.

    You've just restated what laserlight said seven hours past.

    *shrug*

    Well, at least you didn't wait four years.

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

  14. #14
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    O_o

    I can no longer edit the above post.

    For anyone who can't puck up or doesn't know the context: Salem is wrong - not all modern OSs use virtual memory

    Soma
    “Salem Was Wrong!” -- Pedant Necromancer
    “Four isn't random!” -- Gibbering Mouther

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Why does my program crash
    By Diablo667 in forum C Programming
    Replies: 4
    Last Post: 01-26-2012, 12:31 AM
  2. Calling free() causes program crash
    By casmac in forum C Programming
    Replies: 5
    Last Post: 06-12-2010, 07:58 AM
  3. Why does this program crash when I do this?
    By Witch in forum C Programming
    Replies: 4
    Last Post: 03-18-2010, 07:10 PM
  4. my program crash?
    By vrkiller in forum C++ Programming
    Replies: 10
    Last Post: 03-04-2009, 03:03 PM
  5. Program crash ?!
    By tilex in forum C++ Programming
    Replies: 6
    Last Post: 12-21-2004, 08:47 PM

Tags for this Thread