Memory allocation in C application

Question

I'm currently writing a small application to re-familiarise myself with C (it's been a while since I last wrote any) and like most people do, I've run into a memory allocation issue which I cannot figure out.

The code revolves around the setting up of different panels, windows and the associated titles. Precisely it's the allocation of memory for the title string I'm having trouble with.

The base is a structure which contains:

struct TFB_PANEL
{
    WINDOW *window;
    char *title;
};

This is typedef'd in the header file as:

typedef struct TFB_PANEL TfbPanel;

In the related C file I have the following method which initialises an array TFB of a fixed size.

int tfb_init()
{
    if (!_initialised) {
        return -1;
    }

    int i;
    for (i = 0; i < TYPE_MAX; i++) {
        TFB[i] = malloc(sizeof(TfbPanel*));
        TFB[i]->title = calloc((strlen(TYPES[i]) + 18), sizeof(char*));
        switch(i)
        {
            case A:
                sprintf(TFB[i]->title, " %s | r | h | s | t ", TYPES[i]);
                break;
            case B:
                sprintf(TFB[i]->title, " f | %s | h | s | t ", TYPES[i]);
                break;
            case C:
                sprintf(TFB[i]->title, " f | r | %s | s | t ", TYPES[i]);
                break;
            case D:
                sprintf(TFB[i]->title, " f | r | h | %s | t ", TYPES[i]);
                break;
            case E:
                sprintf(TFB[i]->title, " f | r | h | s | %s ", TYPES[i]);
                break;
        }

        TFB[i]->window = tfb_create_window(i);
    }
    return 0;
}

Now the error occurs when C is initialised. A and B get set correctly to a length of 25 characters. C on the other hand should contain 24 characters after initialisation but instead (from within tfb_create_window) I recieve

Program received signal EXC_BAD_ACCESS, Could not access memory
Reason: 13 at address 0x0000000000000000
0x00007fff930b9390 in strcmp()

Examining the stack shows TFB[C] being initialised correctly but the title element is not. This contains a null element as if I had never made the call to calloc.

Please can someone explain where I am going wrong with this or why A and B get initialised correctly but C kills the app. Everything was going beautifully until about 6am this morning, it's been down hill since then.

If it helps, tfb_create_window is defined with a call to _create_window and a call to tfb_get_title as follows:

char *tfb_get_title(int type)
{
    if (type >= TFB_MAX) {
        return (char*)NULL;
    }
    return TFB[type]->title;
}

WINDOW *tfb_create_window(int type)
{
    int height = ((LINES - WIN_OFFSET_Y) / 3);
    char *title = tfb_get_title(type);
    return _create_window(height, WIN_SIDEBAR_X, WIN_OFFSET_Y, 0, COLOUR_MAIN, title);
}

WINDOW *_create_window(int height, int width, int starty, int startx, int color, const char *title)
{
    WINDOW *window;
    window = newwin(height, width, starty, startx);
    box(window, 0, 0);
    mvwprintw(window, 0, 2, title);
    wbkgd(window, COLOR_PAIR(color));
    return window;
}

This isn't your problem, but note that you're actually allocating too much memory at one point: calloc((strlen(TYPES[i]) + 18), sizeof(char*)). This allocates space for (strlen(TYPES[i]) + 18 char pointers, which is probably 4 or 8 times as much memory as you really need. — svk
– svk, Commented Sep 14, 2013 at 9:53
I thought as much, I was originally allocating only strlen(TYPES[i]) + 18 but I added the multiplier whilst trying to fix it. I've also tried just using malloc(strlen(TYPES[i]) + 18) but to no avail. — mproffitt
– mproffitt, Commented Sep 14, 2013 at 9:55
@mplf Are you sure you want to do this malloc(sizeof(TfbPanel*)) instead of malloc(sizeof(TfbPanel)) ? Also, what is TFB[i]? — Uchia Itachi
– Uchia Itachi, Commented Sep 14, 2013 at 10:13
Memory for TfbPanel isn't the issue, regardless of which way round I allocate memory for the panel I still get the exact same bad access signal from the string comparison. TFB[i] is a pointer to a valid block of memory which previously was working fine until I added the dynamic title in. The TFB array is defined as: TfbPanel *TFB[TYPE_MAX] — mproffitt
– mproffitt, Commented Sep 14, 2013 at 10:20
I mean you want the size of pointer to be your size of TfbPanel object? — Uchia Itachi
– Uchia Itachi, Commented Sep 14, 2013 at 10:23

cmaster - reinstate monica · Accepted Answer · 2013-09-14 10:27:56Z

1

There is a number of bugs in this code, the most severe is, that you use sizeof(Type*) instead of sizeof(Type). The later is the actual size of the object, the first is just the size of a pointer to that object (which is 8 bytes on 64 bit machines).

That means, that malloc(sizeof(TfbPanel*)) allocated too little memory for your TfbPanel, so the rest of the program has undefined behaviour.

The calloc() is wrong in the same way, but uncritical, because you allocate eight times as much memory as you need.

However, the calloc() call has another bug: you add 18 to the result of strlen(), which is the number of payload characters you add in your sprintf() calls. Which is one too few, because the result of strlen() does not include the null byte.

If you can use a POSIX-2008 compliant libc (like the glibc on linux systems), you can use asprintf() instead of sprintf(), it will automatically malloc enough space for the resulting string, avoiding any possible buffer size errors. If you can't use that function, use at least snprintf() to avoid accessing unallocated memory.

answered Sep 14, 2013 at 10:27

cmaster - reinstate monica

41.1k9 gold badges69 silver badges110 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

mproffitt Over a year ago

Based on the comments so far I'd already swapped out the assignment of malloc(sizeof(TfbPanel*)) to use malloc(sizeof(struct TfbPanel)) - the key issue as you rightly pointed out was 1 too few characters to the string. I've added this in and it's resolved. Thank you.

ioseph · Accepted Answer · 2013-09-14 09:56:54Z

0

If you step through with gdb to this line:

sprintf(TFB[i]->title, " f | r | %s | s | t ", TYPES[i]);

What is the values of TYPES[i]?

answered Sep 14, 2013 at 9:56

ioseph

1,9171 gold badge21 silver badges26 bronze badges

3 Comments

mproffitt Over a year ago

TYPES is a fixed array defined as char *TYPE[] = {"feature", "release", "hotfix", "support", "trunk"};

varnie Over a year ago

what is the value of TYPE_MAX?

mproffitt Over a year ago

enum ranging from A - TYPE_MAX corresponding to the key values from the TYPE array

Collectives™ on Stack Overflow

Memory allocation in C application

2 Answers 2

1 Comment

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related