Segmentation fault in C program using array of char * and malloc

Question

Silly little C program, but for the life of me I can't figure out why I keep getting a segmentation fault. I believe I'm allocating enough memory with malloc. Any help would be much appreciated. The code is supposed to read a day of the month and 'reminder' string from the user, and then reads these into data pointed to by the array reminders and allocated memory space with malloc.

/* Prints a one-month reminder list (dynamic string version) */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_REMIND 50           /* maximum number of reminders */
#define MSG_LEN 60              /* maximum length of reminder message */

int read_line(char str[], int n);

int main(void)
{
    char *reminders[MAX_REMIND];
    char day_str[3], msg_str[MSG_LEN+1];
    int day, i, j, num_remind = 0;

    for (;;) {
        if (num_remind == MAX_REMIND) {
            printf("-- no space left --\n");
            break;
        }

        printf("Enter day and reminder: ");
        scanf("%2d", &day);
        if (day == 0) {
            break;
        } 
        sprintf(day_str, "%2d", day);
        read_line(msg_str, MSG_LEN);

        for (i = 0; i < num_remind; i++) {
            if (strcmp(day_str, reminders[i]) < 0) {
                break;
            }
        }
        for (j = num_remind; j > i; j--) {
            reminders[j] = reminders[j-1];
        }

        reminders[i] = malloc(2 + strlen(msg_str) + 10);
        if (reminders[i] = NULL) {
            printf("-- No Space Left --\n");
            break;
        }

        strcpy(reminders[i], day_str);
        strcat(reminders[i], msg_str);

        num_remind++;
    }

    printf("\nDay Reminder\n");
    for (i = 0; i < num_remind; i++) {
        printf(" %s\n", reminders[i]);
    }

    return 0;
}
int read_line(char str[], int n) {
    int ch, i = 0;

    while ((ch = getchar()) != '\n') {
        if (i < n) {
            str[i++] = ch;
        }
    }
    str[i] = '\0';
    return i;
}

You don't seem to check day, except for zero. Suppose day is -10, or 100, or anything else that will result in more than two characters when printed. I know that those are not valid days, but never trust input. — Sjoerd
– Sjoerd, Commented Dec 15, 2011 at 7:36
"for the life of me I can't figure out why I keep getting a segmentation fault" -- It can be tough if all you're doing is staring at the code and you're not an experienced C programmer for whom = in an if statement jumps out. But any debugger or IDE will tell you that it segfaults in strcpy and the first argument to that call is 0. — Jim Balter
– Jim Balter, Commented Dec 15, 2011 at 10:02

David Schwartz · Accepted Answer · 2011-12-15 07:35:44Z

2

    if (reminders[i] = NULL) {

You set reminders[i] to NULL before you dereference it.

answered Dec 15, 2011 at 7:35

David Schwartz

184k18 gold badges229 silver badges293 bronze badges

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

Comments

dreamlax · Accepted Answer · 2011-12-15 07:37:32Z

1

The dreaded assignment/comparison typo! This is a hint that your compiler warning levels are not set high enough or that you are ignoring the warnings.

// Should be '==' not '='
if (reminders[i] = NULL) {
    printf("-- No Space Left --\n");
    break;
}

Also, your read_line function is similar to fgets, which may simplify things.

Lastly, always validate your user input. Make sure scanf returns the number of items that you've asked for (usually 1 for simple one-item input). Otherwise, you're likely to venture into undefined behaviour.

answered Dec 15, 2011 at 7:37

dreamlax

95.7k31 gold badges166 silver badges211 bronze badges

Comments

zed_0xff · Accepted Answer · 2011-12-15 07:40:24Z

0

wrong:

if (reminders[i] = NULL) {

right:

if (reminders[i] == NULL) {

also, you are concatenating day_str & msg_str, so it's better to alloc:

reminders[i] = malloc(2 + strlen(msg_str) + strlen(day_str));

answered Dec 15, 2011 at 7:40

zed_0xff

33.5k8 gold badges55 silver badges72 bronze badges

Comments

Majid Azimi · Accepted Answer · 2011-12-15 07:39:30Z

0

You are doing assignment in if clause. It should be like this:

if (reminders[i] == NULL) {
    printf("-- No Space Left --\n");
    break;
}

answered Dec 15, 2011 at 7:39

Majid Azimi

5,77513 gold badges69 silver badges122 bronze badges

2 Comments

user641687 Over a year ago

Yep, thanks guys. I really need to read my own code more carefully.

Jim Balter Over a year ago

@MassStrike What you really need is a decent compiler or lint that detects such errors. You also need some basic debugging skills ... the segfault happens in strcpy and it's trivial to note that its argument is 0.

Collectives™ on Stack Overflow

Segmentation fault in C program using array of char * and malloc

4 Answers 4

Comments

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related